On 8/22/19 12:14 PM, Dave Chinner wrote: > On Thu, Aug 22, 2019 at 11:10:57AM +0200, Peter Zijlstra wrote: >> >> Ah, current_gfp_context() already seems to transfer PF_MEMALLOC_NOFS >> into the GFP flags. >> >> So are we sure it is broken and needs mending? > > Well, that's what we are trying to work out. The problem is that we > have code that takes locks and does allocations that is called both > above and below the reclaim "lock" context. Once it's been seen > below the reclaim lock context, calling it with GFP_KERNEL context > above the reclaim lock context throws a deadlock warning. > > The only way around that was to mark these allocation sites as > GFP_NOFS so lockdep is never allowed to see that recursion through > reclaim occur. Even though it isn't a deadlock vector. > > What we're looking at is whether PF_MEMALLOC_NOFS changes this - I > don't think it does solve this problem. i.e. if we define the > allocation as GFP_KERNEL and then use PF_MEMALLOC_NOFS where reclaim > is not allowed, we still have GFP_KERNEL allocations in code above > reclaim that has also been seen below relcaim. And so we'll get > false positive warnings again. If I understand both you and the code directly, the code sites won't call __fs_reclaim_acquire when called with current->flags including PF_MEMALLOC_NOFS. So that would mean they "won't be seen below the reclaim" and all would be fine, right? > What I think we are going to have to do here is manually audit > each of the KM_NOFS call sites as we remove the NOFS from them and > determine if ___GFP_NOLOCKDEP is needed to stop lockdep from trying > to track these allocation sites. We've never used this tag because > we'd already fixed most of these false positives with explicit > GFP_NOFS tags long before ___GFP_NOLOCKDEP was created. > > But until someone starts doing the work, I don't know if it will > work or even whether conversion PF_MEMALLOC_NOFS is going to > introduce a bunch of new ways to get false positives from lockdep... > > Cheers, > > Dave. >