Re: pfind_locked(pid) fails when in a jail?

On Tue, 2017-10-17 at 00:08 +0000, Rick Macklem wrote:
> [stuff snipped]
> > 
> > > 
> > > > 
> > > > 
> > > pfind* does not do any filtering.
> > > 
> Hmm, well I have no idea why the jailed mounts get looping in here then.
> 
> > 
> > > 
> > > The real question though is why are you calling it in the first place. The
> > > calls
> > > I grepped in nfscl_procdoesntexist are highly suspicious - there is no
> > > guarantee
> > > the process you found here is the same you had at the time you were saving
> > > the pid.
> > > 
> Long long ago (about 2002) this code was written for OpenBSD2.6. I added
> a call from the kernel exit() code to do this. When I ported it to FreeBSD
> around 2005, I didn't find any way for a process exit notification to be done,
> so I created the first version of this code. (This way of doing it was first
> coded for Mac OS X 10.3, if I recall correctly.)
> 
> Since it does check that the time of process creation is the same, it doesn't
> seem likely that it would find a different process (ie. two processes with the
> same pid that were created at the same time within the clock resolution of
> that creation time seems highly unlikely in practice?).
> 
> > 
> > > 
> > > There is no usable process exit notification right now, but it can be added
> > > if necessary.
> > > 
> If there was a way for the NFS client to register to get a notification that a
> given process is terminating (exit'ng), that could certainly be used instead
> of this code.
> 
> I wouldn't want a call for every exit(), but only the ones that have NFSv4 opens.
> 
> > 
> > > 
> > > 
> > > Does that mean there is something wrong with the existing eventhandler
> > > notifications related to proc fork/exec/exit?
> > > 
> > I already noted in the other mail that the current mechanism has
> > avoidable global locking, lists traversals etc. But even with these
> > issues fixed it calls everything for everyone.
> > 

All of that locking-and-lookup overhead in the event notification
system is already happening on every process exit, whether anyone has
registered an event handler or not.  If you want to actually avoid the
avoidable overheads, the answer is to fix the existing code, not create
another new mechanism that adds more overhead without addressing the
existing problem.  I suspect the worst of the existing overhead can be
eliminated with some fairly small changes, and I hope to find time to
look at it later this week.

On the issue of filtering the callbacks to just the exits you're
interested in... it doesn't seem any better to me to do the filtering
at the source unless there are multiple registered handlers at once
that all need the same filtering.  How many different things in the
kernel would want process-exit filtering based on "have NFSv4 opens"?
 That seems like a single-consumer kind of filter.

-- Ian

> > What's needed is a mechanism to register per-process callbacks. Details
> > can be flamed over (e.g. should it require allocating a buffer per
> > process or perhaps just one and then point to it from a resizable
> > per-proc table when registered), but calling something which has nothing
> > to do in almost all cases and from in a super inefficient way at that is
> > definitely something we need to start cleaning up.
> Yes, I would agree, although it doesn't explain what this CPU hog case is
> caused by.
> 
> Thanks for the comments and if you create/commit the above, let me know
> and I'll change the NFS client code to use it (if your patch doesn't do that).
> 
> rick
> 
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd
> .org"