linux-kernel - Re: [PATCH] xfs: use GFP_NOFS argument in radix_tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150323052449.GO28621@dastard>
Date:	Mon, 23 Mar 2015 16:24:49 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Sanidhya Kashyap <sanidhya.gatech@...il.com>
Cc:	xfs@....sgi.com, linux-kernel@...r.kernel.org, taesoo@...ech.edu,
	changwoo@...ech.edu, sanidhya@...ech.edu, blee@...ech.edu,
	csong84@...ech.edu
Subject: Re: [PATCH] xfs: use GFP_NOFS argument in radix_tree_preload

On Mon, Mar 23, 2015 at 01:06:23AM -0400, Sanidhya Kashyap wrote:
> From: Byoungyoung Lee <blee@...ech.edu>
> 
> 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@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/