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 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..... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
