Re: [PATCH] xfs: use GFP_NOFS argument in radix_tree_preload

[Dave Chinner]

On Mon, Mar 23, 2015 at 01:06:23AM -0400, Sanidhya Kashyap wrote:
> From: Byoungyoung Lee <blee@xxxxxxxxxx>
> 
> Following the convention of other file systems, GFP_NOFS
> should be used as an argument for radix_tree_preload() instead
> of GFP_KERNEL.

"convention of other filesystems" is not a reason for changing from
GFP_KERNEL to GFP_NOFS. There are rules for when GFP_NOFS needs to
be used, and so we only need to change the code if one of those
rules are triggered. i.e. inside a transaction, holding a lock that
memory reclaim might require to make progress (e.g. ip->i_ilock,
buffer locks, etc). The context in which the allocation is made will
tell you whether GFP_KERNEL is safe or not.

So while the change probably needs to be made, it needs to be made
for the right reasons. I haven't looked at the code, but I have
a pretty good idea of the context the allocation is being made
under. I'd suggest documenting the call path down to
xfs_mru_cache_insert(), because that will tell you exactly what
context the allocaiton is being made in and hence tell everyone else
the real reason we need to make this change...

Call me picky, pendantic and/or annoying, but if you are looking at
validating/correcting allocation flags then you need to understand
the rules and context in which the allocation is being made...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/