> On Sep 30, 2022, at 10:13 AM, Robert Elz <k...@munnari.oz.au> wrote: > > Date: Thu, 29 Sep 2022 16:47:06 -0000 (UTC) > From: chris...@astron.com (Christos Zoulas) > Message-ID: <th4i6a$aq$1...@ciao.gmane.io> > > | 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
signature.asc
Description: Message signed with OpenPGP