On Fri 03-06-16 09:22:54, Dave Chinner wrote: > On Thu, Jun 02, 2016 at 05:46:19PM +0200, Michal Hocko wrote: > > On Thu 02-06-16 17:11:16, Peter Zijlstra wrote: > > > 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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
