On Tue, Jun 21, 2016 at 04:26:28PM +0200, Michal Hocko wrote: > On Wed 15-06-16 17:21:54, Dave Chinner wrote: > [...] > > Hopefully you can see the complexity of the issue - for an allocation > > in the bmap btree code that could occur outside both inside and > > outside of a transaction context, we've got to work out which of > > those ~60 high level entry points would need to be annotated. And > > then we have to ensure that in future we don't miss adding or > > removing an annotation as we change the code deep inside the btree > > implementation. It's the latter that is the long term maintainence > > problem the hihg-level annotation approach introduces. > > Sure I can see the complexity here. I might still see this over > simplified but I originally thought that the annotation would be used at > the highest level which never gets called from the transaction or other > NOFS context. So all the layers down would inherit that automatically. I > guess that such a place can be identified from the lockdep report by a > trained eye. Which, as I said before, effectively becomes "turn off lockdep reclaim context checking at all XFS entry points". Yes, we could do that, but it's a "big hammer" solution and there are more entry points than there are memory allocations that need annotations.... > > > > 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... > > > > There are allocations outside transaction context which need to be > > GFP_NOFS - this is what KM_NOFS was originally intended for. > > Is it feasible to mark those by the scope NOFS api as well and drop > the direct KM_NOFS usage? This should help to identify those that are > lockdep only and use the annotation to prevent from the false positives. I don't understand what you are suggesting here. This all started because we use GFP_NOFS in a handful of places to shut up lockdep and you didn't want us to use GFP_NOFS like that. Now it sounds to me like you are advocating setting unconditional GFP_NOFS allocation contexts for entire XFS code paths - whether it's necessary or not - to avoid problems with lockdep false positives. I'm clearly not understanding something here.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
