Re: [PATCH] filelock: fix deadlock detection in POSIX locking

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2024-02-19 at 09:28 +1100, NeilBrown wrote:
> On Mon, 19 Feb 2024, Jeff Layton wrote:
> > The FL_POSIX check in __locks_insert_block was inadvertantly broken
> > recently and is now inserting only OFD locks instead of only legacy
> > POSIX locks.
> > 
> > This breaks deadlock detection in POSIX locks, and may also be the root
> > cause of a performance regression noted by the kernel test robot.
> > Restore the proper sense of the test.
> > 
> > Fixes: b6be3714005c ("filelock: convert __locks_insert_block, conflict and deadlock checks to use file_lock_core")
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Closes: https://lore.kernel.org/oe-lkp/202402181229.f8147f40-oliver.sang@intel.com
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Disregard what I said earlier about this bug being harmless. It broke
> > deadlock detection in POSIX locks (LTP fcntl17 shows the bug). This
> > patch fixes it. It may be best to squash this into the patch that
> > introduced the regression.
> > 
> > I'm not certain if this fixes the performance regression that the KTR
> > noticed recently in this patch, but that's what got me looking more
> > closely, so I'll give it credit for reporting this. Hopefully it'll
> > confirm that result for us.
> > ---
> >  fs/locks.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/locks.c b/fs/locks.c
> > index 26d52ef5314a..90c8746874de 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -812,7 +812,7 @@ static void __locks_insert_block(struct file_lock_core *blocker,
> >  	list_add_tail(&waiter->flc_blocked_member,
> >  		      &blocker->flc_blocked_requests);
> >  
> > -	if ((blocker->flc_flags & (FL_POSIX|FL_OFDLCK)) == (FL_POSIX|FL_OFDLCK))
> > +	if ((blocker->flc_flags & (FL_POSIX|FL_OFDLCK)) == FL_POSIX)
> >  		locks_insert_global_blocked(waiter);
> 
> I wonder how that happened... sorry I didn't notice it in my review.
> 
> Reviewed-by: NeilBrown <neilb@suse.de>
> 

Mea culpa.

I had this bug in the original version of the series, fixed it and then
reverted that fix by accident while rebasing to clean up and reorganize
things.

-- 
Jeff Layton <jlayton@kernel.org>