> On Sep 30, 2022, at 10:13 AM, Robert Elz <kre%munnari.OZ.AU@localhost> wrote: > > Date: Thu, 29 Sep 2022 16:47:06 -0000 (UTC) > From: christos%astron.com@localhost (Christos Zoulas) > Message-ID: <th4i6a$aq$1%ciao.gmane.io@localhost> > > | I think that the way to go is to: > | > | 1. Do the fd_set_exclose() in fd_affix(). That will remove most of the calls > | to fd_set_exclose() *and* the open-coded versions of it. > | 2. Move the open_setfp locking initialization code to fd_affix() and do it > | if fp->f_type == DTYPE_VNODE. This should enable locking in all the > | appropriate cloners. > > I initially intended to reply and say that decisions where to put stuff > like that were for someone else (you, dholland, ...) rather than me, as > I haven't played around much at this level since before vnodes existed. > > But I have been thinking about it, and I disagree with that approach. > > fp_affix() has a job to do, and should be left to do it, without being > burdened by applying weird side effects, sometimes. The "do one thing > and do it well" philosophy applies to more than the commands. > > eg: currently fd_affix() is a void func, but to handle the lock flags > it would need to be able to fail, and return an error code. It would > also need to be able to sleep. That's just wrong. > > O_CLOEXEC and O_??LOCK are high level open() flags, and deserve to be > handled somewhere near the upper levels of the open syscall handling, > not buried in some utility function. That is the feedback that I wanted. But there were two parts to it. How about handling exclose there? It is just making sure that the value from flags is propagated to the exclose field. christos
Attachment:
signature.asc
Description: Message signed with OpenPGP