On Fri, Nov 24, 2017 at 09:03:28PM +0900, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > Thanks. Updated patch below
> > ---
> > From 1009db61988c48c9a9e327a9d076945b29b02eee Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mho...@suse.com>
> > Date: Thu, 23 Nov 2017 17:13:40 +0100
> > Subject: [PATCH] xfs: fortify xfs_alloc_buftarg error handling
>
> Do we need below patch on top of Michal's patch?
> KM_NOFS was added by commit b17cb364dbbbf65a ("xfs: fix missing KM_NOFS
> tags to keep lockdep happy"). If not needed, some comment is expected.
Quite frankly, if the fix is "sprinkle magic undocumented
memalloc_nofs_save() calls around", then you need to think a little
more about the things you just read and the context we're operating
on here.
IOWs:
> ---
> fs/xfs/xfs_buf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 4c6e86d..b73fc76 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1806,6 +1806,7 @@ struct xfs_buf *
> struct dax_device *dax_dev)
> {
> xfs_buftarg_t *btp;
> + unsigned int nofs_flag = memalloc_nofs_save();
>
> btp = kmem_zalloc(sizeof(*btp), KM_SLEEP | KM_NOFS);
xfs_alloc_buftarg() isn't called from transaction context, so this
KM_NOFS flag wasn't added to prevent reclaim deadlocks - it was
added to avoid stupid lockdep false positives (as was stated in the
commit you quoted).
IOWs, GFP_KERNEL allocations in this function used to trigger
lockdep false positives.
So - think for a minute rather than bashing on the keyboard. Why
aren't the other GFP_KERNEL allocations from this function causing
lockdep to trigger warnings?
Yeah - lockdep is a lot smarter these days and the false positive
trigger has clearly been fixed. i.e. there's no false positive
detection occurring here any more under GFP_KERNEL allocations,
so we don't need the KM_NOFS flag anymore.
IOWs, we don't actually need to touch this code, but if you really
must, just remove the KM_NOFS tag.
-Dave.
--
Dave Chinner
da...@fromorbit.com
