Xfs lockdep warning with for-dave-for-4.6 branch

[Michal Hocko]

On Mon 16-05-16 12:41:30, Peter Zijlstra wrote:
> On Fri, May 13, 2016 at 06:03:41PM +0200, Michal Hocko wrote:
[...]
> > So, because the reclaim annotations overload the interrupt level
> > detections and it's seen the inode ilock been taken in reclaim
> > ("interrupt") context, this triggers a reclaim context warning where
> > it thinks it is unsafe to do this allocation in GFP_KERNEL context
> > holding the inode ilock...
> > "
> > 
> > This sounds like a fundamental problem of the reclaim lock detection.
> > It is really impossible to annotate such a special usecase IMHO unless
> > the reclaim lockup detection is reworked completely.
> 
> How would you like to see it done? The interrupt model works well for
> reclaim because how is direct reclaim from a !GFP_NOWAIT allocation not
> an 'interrupt' like thing?

Unfortunately I do not have any good ideas. It would basically require
to allow marking the lockdep context transaction specific AFAIU somehow
and tell that there is no real dependency between !GFP_NOWAIT and
'interrupt' context.
IIRC Dave's emails they have tried that by using lockdep classes and
that turned out to be an overly complex maze which still doesn't work
100% reliably.

> > Until then it
> > is much better to provide a way to add "I know what I am doing flag"
> > and mark problematic places. This would prevent from abusing GFP_NOFS
> > flag which has a runtime effect even on configurations which have
> > lockdep disabled.
> 
> So without more context; no. The mail you referenced mentions:
> 
> "The reclaim -> lock context that it's complaining about here is on
> an inode being reclaimed - it has no active references and so, by
> definition, cannot deadlock with a context holding an active
> reference to an inode ilock. Hence there cannot possibly be a
> deadlock here, but we can't tell lockdep that easily in any way
> without going back to the bad old ways of creating a new lockdep
> class for inode ilocks the moment they enter ->evict. This then
> disables "entire lifecycle" lockdep checking on the xfs inode ilock,
> which is why we got rid of it in the first place."
> 
> But fails to explain the problems with the 'old' approach.
> 
> So clearly this is a 'problem' that has existed for quite a while, so I
> don't see any need to rush half baked solutions either.

Well, at least my motivation for _some_ solution here is that xfs has
worked around this deficiency by forcing GFP_NOFS also for contexts which
are perfectly OK to do __GFP_FS allocation. And that in turn leads to
other issues which I would really like to sort out. So the idea was to
give xfs another way to express that workaround that would be a noop
without lockdep configured.
 
> Please better explain things.
-- 
Michal Hocko
SUSE Labs