Re: linux-next: kernel BUG at mm/slub.c:1447!
From: Michal Hocko
Date: Fri Oct 02 2015 - 03:25:31 EST
On Thu 01-10-15 13:49:04, Andrew Morton wrote:
[...]
> mpage_readpage() is getting the __GFP_HIGHMEM from mapping_gfp_mask()
> and that got passed all the way into kmem_cache_alloc() to allocate a
> bio. slab goes BUG if asked for highmem.
>
> A fix would be to mask off __GFP_HIGHMEM right there in
> mpage_readpage().
Yes, this is an obvious bug in the patch. It should only make the gfp
mask more restrictive.
> But I think the patch needs a bit of a rethink. mapping_gfp_mask() is
> the mask for allocating a file's pagecache. It isn't designed for
> allocation of memory for IO structures, file metadata, etc.
>
> Now, we could redefine mapping_gfp_mask()'s purpose (or formalize
> stuff which has been sneaking in anyway). Treat mapping_gfp_mask() as
> a constraint mask - instead of it being "use this gfp for this
> mapping", it becomes "don't use these gfp flags for this mapping".
>
> Hence something like:
>
> gfp_t mapping_gfp_constraint(struct address_space *mapping, gfp_t gfp_in)
> {
> return mapping_gfp_mask(mapping) & gfp_in;
> }
>
> So instead of doing this:
>
> @@ -370,12 +371,13 @@ mpage_readpages(struct address_space *ma
> prefetchw(&page->flags);
> list_del(&page->lru);
> if (!add_to_page_cache_lru(page, mapping,
> - page->index, GFP_KERNEL)) {
> + page->index,
> + gfp)) {
>
> Michal's patch will do:
>
> @@ -370,12 +371,13 @@ mpage_readpages(struct address_space *ma
> prefetchw(&page->flags);
> list_del(&page->lru);
> if (!add_to_page_cache_lru(page, mapping,
> - page->index, GFP_KERNEL)) {
> + page->index,
> + mapping_gfp_constraint(mapping, GFP_KERNEL))) {
>
> ie: use mapping_gfp_mask() to strip out any GFP flags which the
> filesystem doesn't want used. If the filesystem has *added* flags to
> mapping_gfp_mask() then obviously this won't work and we'll need two
> fields in the address_space or something.
>
> Meanwhile I'll drop "mm, fs: obey gfp_mapping for add_to_page_cache()",
> thanks for the report.
mapping_gfp_mask is used at many places so I think it would be better to
fix this particular place (others seem to be correct). It would make the
stable backport much easier. We can build a more sane API on top. What
do you think?
Here is the respin of the original patch. I will post another one which
will add mapping_gfp_constraint on top. It will surely be less error
prone.
---