Re: [Resend] 3.2-rc6+: Reported regressions from 3.0 and 3.1

[Linus Torvalds <torvalds@linux-foundation.org>]

On Tue, Dec 20, 2011 at 8:23 PM, Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
>
> I don't think this is a regression.  It's been seen before, but the
> patch never got submitted, or was lost somewhere. I believe this
> will fix it.

Hmm. This patch looks obviously correct. But it looks *so* obviously
correct that it just makes me suspicious - this is not new or seldom
used code, it's been this way for ages and used all the time. That
line literally goes back to 2007, commit eb2be189317d0. And it looks
like even before that we had a GFP_KERNEL for the add_to_page_cache()
case and that goes back to before the git history. So this is
*ancient*.

Maybe almost nobody uses __read_cache_page() with a non-GFP_KERNEL gfp
and as a result we've not noticed.

Or maybe there is some crazy reason why it calls "add_to_page_cache()"
with GFP_KERNEL.

Adding the usual suspects for mm/filemap.c to the cc line (Andrew is
already cc'd, but Al and Hugh should comment).

Ack's, people? Is it really as obvious as it looks, and we've just had
this bug forever?

            Linus

--- snip snip ---
> vfs: __read_cache_page should use gfp argument rather than GFP_KERNEL
>
> lockdep reports a deadlock in jfs because a special inode's rw semaphore
> is taken recursively. The mapping's gfp mask is GFP_NOFS, but is not used
> when __read_cache_page() calls add_to_page_cache_lru().
>
> Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index c106d3b..c9ea3df 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1828,7 +1828,7 @@ repeat:
>                page = __page_cache_alloc(gfp | __GFP_COLD);
>                if (!page)
>                        return ERR_PTR(-ENOMEM);
> -               err = add_to_page_cache_lru(page, mapping, index, GFP_KERNEL);
> +               err = add_to_page_cache_lru(page, mapping, index, gfp);
>                if (unlikely(err)) {
>                        page_cache_release(page);
>                        if (err == -EEXIST)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>