Re: Xfs lockdep warning with for-dave-for-4.6 branch

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jun 02, 2016 at 04:50:49PM +0200, Michal Hocko wrote:
> On Wed 01-06-16 20:16:17, Peter Zijlstra wrote:

> > So my favourite is the dedicated GFP flag, but if that's unpalatable for
> > the mm folks then something like the below might work. It should be
> > similar in effect to your proposal, except its more limited in scope.
> [...]
> > @@ -2876,11 +2883,36 @@ static void __lockdep_trace_alloc(gfp_t gfp_mask, unsigned long flags)
> >  	if (DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)))
> >  		return;
> >  
> > +	/*
> > +	 * Skip _one_ allocation as per the lockdep_skip_alloc() request.
> > +	 * Must be done last so that we don't loose the annotation for
> > +	 * GFP_ATOMIC like things from IRQ or other nesting contexts.
> > +	 */
> > +	if (current->lockdep_reclaim_gfp & __GFP_SKIP_ALLOC) {
> > +		current->lockdep_reclaim_gfp &= ~__GFP_SKIP_ALLOC;
> > +		return;
> > +	}
> > +
> >  	mark_held_locks(curr, RECLAIM_FS);
> >  }
> 
> I might be missing something but does this work actually? Say you would
> want a kmalloc(size), it would call
> slab_alloc_node
>   slab_pre_alloc_hook
>     lockdep_trace_alloc
> [...]
>   ____cache_alloc_node
>     cache_grow_begin
>       kmem_getpages
>         __alloc_pages_node
> 	  __alloc_pages_nodemask
> 	    lockdep_trace_alloc

Bugger :/ You're right, that would fail.

So how about doing:

#define __GFP_NOLOCKDEP	(1u << __GFP_BITS_SHIFT)

this means it cannot be part of address_space::flags or
radix_tree_root::gfp_mask, but that might not be a bad thing.

And this solves the scarcity thing, because per pagemap we need to have
5 'spare' bits anyway.

> I understand your concerns about the scope but usually all allocations
> have to be __GFP_NOFS or none in the same scope so I would see it as a
> huge deal.

With scope I mostly meant the fact that you have two calls that you need
to pair up. That's not really nice as you can 'annotate' a _lot_ of code
in between. I prefer the narrower annotations where you annotate a
single specific site.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs