Re: [PATCH 07/12] xfs: use __GFP_NOLOCKDEP instead of GFP_NOFS

[Dave Chinner <david@fromorbit.com>]

On Sat, Jun 22, 2024 at 05:44:11PM +0800, Long Li wrote:
> On Tue, Jan 16, 2024 at 09:59:45AM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > In the past we've had problems with lockdep false positives stemming
> > from inode locking occurring in memory reclaim contexts (e.g. from
> > superblock shrinkers). Lockdep doesn't know that inodes access from
> > above memory reclaim cannot be accessed from below memory reclaim
> > (and vice versa) but there has never been a good solution to solving
> > this problem with lockdep annotations.
> > 
> > This situation isn't unique to inode locks - buffers are also locked
> > above and below memory reclaim, and we have to maintain lock
> > ordering for them - and against inodes - appropriately. IOWs, the
> > same code paths and locks are taken both above and below memory
> > reclaim and so we always need to make sure the lock orders are
> > consistent. We are spared the lockdep problems this might cause
> > by the fact that semaphores and bit locks aren't covered by lockdep.
> > 
> > In general, this sort of lockdep false positive detection is cause
> > by code that runs GFP_KERNEL memory allocation with an actively
> > referenced inode locked. When it is run from a transaction, memory
> > allocation is automatically GFP_NOFS, so we don't have reclaim
> > recursion issues. So in the places where we do memory allocation
> > with inodes locked outside of a transaction, we have explicitly set
> > them to use GFP_NOFS allocations to prevent lockdep false positives
> > from being reported if the allocation dips into direct memory
> > reclaim.
> > 
> > More recently, __GFP_NOLOCKDEP was added to the memory allocation
> > flags to tell lockdep not to track that particular allocation for
> > the purposes of reclaim recursion detection. This is a much better
> > way of preventing false positives - it allows us to use GFP_KERNEL
> > context outside of transactions, and allows direct memory reclaim to
> > proceed normally without throwing out false positive deadlock
> > warnings.
> 
> Hi Dave,
> 
> I recently encountered the following AA deadlock lockdep warning
> in Linux-6.9.0. This version of the kernel has currently merged
> your patch set. I believe this is a lockdep false positive warning.

Yes, it is.

> The xfs_dir_lookup_args() function is in a non-transactional context
> and allocates memory with the __GFP_NOLOCKDEP flag in xfs_buf_alloc_pages().
> Even though __GFP_NOLOCKDEP can tell lockdep not to track that particular
> allocation for the purposes of reclaim recursion detection, it cannot
> completely replace __GFP_NOFS.

We are not trying to replace GFP_NOFS with __GFP_NOLOCKDEP. What we
are trying to do is annotate the allocation sites where lockdep
false positives will occur. That way if we get a lockdep report from
a location that uses __GFP_NOLOCKDEP, we know that it is either a
false positive or there is some nested allocation that did not honor
__GFP_NOLOCKDEP.

We've already fixed a bunch of nested allocations (e.g. kasan,
kmemleak, etc) to propagate the __GFP_NOLOCKDEP flag so they don't
generate false positives, either. So the amount of noise has already
been reduced.

> Getting trapped in direct memory reclaim
> maybe trigger the AA deadlock warning as shown below.

No, it can't. xfs_dir_lookup() can only lock referenced inodes.
xfs_reclaim_inodes_nr() can only lock unreferenced inodes. It is not
possible for the same inode to be both referenced and unreferenced
at the same time, therefore memory reclaim cannot self deadlock
through this path.

I expected to see some situations like this when getting rid of
GFP_NOFS (because now memory reclaim runs in places it never used
to). Once I have an idea of the sorts of false positives that are
still being tripped over, I can formulate a plan to eradicate them,
too.

-Dave.
-- 
Dave Chinner
david@fromorbit.com