Re: [PATCH] nfsd: close potential race between delegation break and laundromat

["J. Bruce Fields" <bfields@fieldses.org>]

On Mon, Jul 07, 2014 at 04:00:17PM -0400, Jeff Layton wrote:
> On Mon, 7 Jul 2014 15:31:08 -0400
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Mon, Jul 07, 2014 at 03:14:50PM -0400, Jeff Layton wrote:
> > > Sigh...after staring at this patch all day, I think I now see a
> > > potential problem with the fault injection piece.
> > > 
> > > In the "recall" case, we may find a delegation on the cl_delegations
> > > list that has already been recalled. If that's the case, then we
> > > probably should just skip it.
> > > 
> > > I'll fix this patch, retest and resend. Sorry for the noise...
> > 
> > OK, so sounds like the fix isn't a big deal, but it also sounds like the
> > fault injection code has come up as needing special attention a few
> > times during this work now.
> > 
> > Has anyone actually ended up using that code?  Anna?
> > 
> > In retrospect I wonder if it's worth the trouble.  It's very
> > special-purpose stuff and if nobody's using it then maybe we could just
> > remove it....
> > 
> > --b.
> 
> I certainly wouldn't object. Having printks that are triggered by
> reading from a debugfs file is just weird, and there are quite a few
> races and bugs in this code.
> 
> I have patches that are basically a rewrite of it, but I left the UI
> the same. It was necessary in order to allow for the changes to the
> locking around this code, but that does mean that it's a larger chunk
> of code than it used to be.
> 
> That said, we've shipped this code in several releases now so removing
> it without any warning might not be a good idea. What may be best is to
> mark it for deprecation if we don't want to keep it, and take my patches
> that overhaul it in the interim. Then, remove it altogether in 2-3
> releases.

Well, if you've already done the work, fine.

If it becomes a problem then I wouldn't rule out removing it summarily.
It's purely for client testing, and if neither Anna nor any of us knows
anyone using it then chances are nobody would notice.

--b.