| Message ID | 20170109143903.32280-1-mhocko@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 09, 2017 at 03:39:02PM +0100, Michal Hocko wrote: > From: Michal Hocko <mhocko@suse.com> > > b335b0034e25 ("Btrfs: Avoid using __GFP_HIGHMEM with slab allocator") > has reduced the allocation mask in btrfs_releasepage to GFP_NOFS just > to prevent from giving an unappropriate gfp mask to the slab allocator > deeper down the callchain (in alloc_extent_state). This is wrong for > two reasons a) GFP_NOFS might be just too restrictive for the calling > context b) it is better to tweak the gfp mask down when it needs that. > > So just remove the mask tweaking from btrfs_releasepage and move it > down to alloc_extent_state where it is needed. > > Signed-off-by: Michal Hocko <mhocko@suse.com> > --- > fs/btrfs/extent_io.c | 5 +++++ > fs/btrfs/inode.c | 2 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index b38150eec6b4..f6ae94a4acad 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -226,6 +226,11 @@ static struct extent_state *alloc_extent_state(gfp_t mask) > { > struct extent_state *state; > > + /* > + * The given mask might be not appropriate for the slab allocator, > + * drop the unsupported bits > + */ > + mask &= ~(__GFP_DMA32|__GFP_HIGHMEM); Is this future proof enough? As it's enumerating some gfp flags, what if more are necessary in the future? I'm interested about some synthetic gfp flags that would not require knowledge about what is or is not acceptable for slab allocator. But otherwise looks ok to me, I'm going to merge the patch. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed 11-01-17 14:55:50, David Sterba wrote: > On Mon, Jan 09, 2017 at 03:39:02PM +0100, Michal Hocko wrote: > > From: Michal Hocko <mhocko@suse.com> > > > > b335b0034e25 ("Btrfs: Avoid using __GFP_HIGHMEM with slab allocator") > > has reduced the allocation mask in btrfs_releasepage to GFP_NOFS just > > to prevent from giving an unappropriate gfp mask to the slab allocator > > deeper down the callchain (in alloc_extent_state). This is wrong for > > two reasons a) GFP_NOFS might be just too restrictive for the calling > > context b) it is better to tweak the gfp mask down when it needs that. > > > > So just remove the mask tweaking from btrfs_releasepage and move it > > down to alloc_extent_state where it is needed. > > > > Signed-off-by: Michal Hocko <mhocko@suse.com> > > --- > > fs/btrfs/extent_io.c | 5 +++++ > > fs/btrfs/inode.c | 2 +- > > 2 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > > index b38150eec6b4..f6ae94a4acad 100644 > > --- a/fs/btrfs/extent_io.c > > +++ b/fs/btrfs/extent_io.c > > @@ -226,6 +226,11 @@ static struct extent_state *alloc_extent_state(gfp_t mask) > > { > > struct extent_state *state; > > > > + /* > > + * The given mask might be not appropriate for the slab allocator, > > + * drop the unsupported bits > > + */ > > + mask &= ~(__GFP_DMA32|__GFP_HIGHMEM); > > Is this future proof enough? As it's enumerating some gfp flags, what if > more are necessary in the future? I'm interested about some synthetic > gfp flags that would not require knowledge about what is or is not > acceptable for slab allocator. Well, I agree, that something like slab_restrict_gfp_mask(gfp_t gfp_mask) would be much better. And in fact that sounds like a nice future cleanup. I haven't checked how many users would find it useful yet but I am putting that on my todo list. > But otherwise looks ok to me, I'm going to merge the patch. Thanks. Thanks!
On Wed 11-01-17 14:55:50, David Sterba wrote:
[...]
> But otherwise looks ok to me, I'm going to merge the patch. Thanks.
I have only now noticed typo in the subject. s@etrfs:@btrfs:@
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index b38150eec6b4..f6ae94a4acad 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -226,6 +226,11 @@ static struct extent_state *alloc_extent_state(gfp_t mask) { struct extent_state *state; + /* + * The given mask might be not appropriate for the slab allocator, + * drop the unsupported bits + */ + mask &= ~(__GFP_DMA32|__GFP_HIGHMEM); state = kmem_cache_alloc(extent_state_cache, mask); if (!state) return state; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index baa40d34d2c9..d118d4659c28 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -8994,7 +8994,7 @@ static int btrfs_releasepage(struct page *page, gfp_t gfp_flags) { if (PageWriteback(page) || PageDirty(page)) return 0; - return __btrfs_releasepage(page, gfp_flags & GFP_NOFS); + return __btrfs_releasepage(page, gfp_flags); } static void btrfs_invalidatepage(struct page *page, unsigned int offset,