On Fri 03-06-16 09:22:54, Dave Chinner wrote:

On Thu, Jun 02, 2016 at 05:46:19PM +0200, Michal Hocko wrote:

quoted

On Thu 02-06-16 17:11:16, Peter Zijlstra wrote:

quoted

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.

Yes, I can see you point. What I meant to say is that we would most
probably end up with the following pattern
	lockdep_trace_alloc_enable()
	some_foo_with_alloc(gfp_mask);
	lockdep_trace_alloc_disable()

and some_foo_with_alloc might be a lot of code.

That's the problem I see with this - the only way to make it
maintainable is to precede each enable/disable() pair with a comment
explaining *exactly* what those calls are protecting.  And that, in
itself, becomes a maintenance problem, because then code several
layers deep has no idea what context it is being called from and we
are likely to disable warnings in contexts where we probably
shouldn't be.

I am not sure I understand what you mean here. I thought the problem is
that:

func_A (!trans. context)		func_B (trans. context)
 foo1()					  foo2()
   bar(inode, GFP_KERNEL)		    bar(inode, GFP_NOFS)

so bar(inode, gfp) can be called from two different contexts which
would confuse the lockdep. And the workaround would be annotating bar
depending on the context it is called from - either pass a special gfp
flag or do disable/enable thing. In both cases that anotation should be
global for the whole func_A, no? Or is it possible that something in
that path would really need a reclaim lockdep detection?

I think such an annotation approach really requires per-alloc site
annotation, the reason for it should be more obvious from the
context. e.g. any function that does memory alloc and takes an
optional transaction context needs annotation. Hence, from an XFS
perspective, I think it makes more sense to add a new KM_ flag to
indicate this call site requirement, then jump through whatever
lockdep hoop is required within the kmem_* allocation wrappers.
e.g, we can ignore the new KM_* flag if we are in a transaction
context and so the flag is only activated in the situations were
we currently enforce an external GFP_NOFS context from the call
site.....

Hmm, I thought we would achive this by using the scope GFP_NOFS usage
which would mark those transaction related conctexts and no lockdep
specific workarounds would be needed...

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>