linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Minchan Kim <minchan@kernel.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kosaki.motohiro@gmail.com, rientjes@google.com,
	Neil Brown <neilb@suse.de>,
	Artem Bityutskiy <dedekind1@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Steven Whitehouse <swhiteho@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	James Morris <jmorris@namei.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Sage Weil <sage@newdream.net>
Subject: Re: [PATCH] vmalloc: add warning in __vmalloc
Date: Wed, 2 May 2012 12:46:10 -0700	[thread overview]
Message-ID: <20120502124610.175e099c.akpm@linux-foundation.org> (raw)
In-Reply-To: <1335932890-25294-1-git-send-email-minchan@kernel.org>

On Wed,  2 May 2012 13:28:09 +0900
Minchan Kim <minchan@kernel.org> wrote:

> Now there are several places to use __vmalloc with GFP_ATOMIC,
> GFP_NOIO, GFP_NOFS but unfortunately __vmalloc calls map_vm_area
> which calls alloc_pages with GFP_KERNEL to allocate page tables.
> It means it's possible to happen deadlock.
> I don't know why it doesn't have reported until now.
> 
> Firstly, I tried passing gfp_t to lower functions to support __vmalloc
> with such flags but other mm guys don't want and decided that
> all of caller should be fixed.
> 
> http://marc.info/?l=linux-kernel&m=133517143616544&w=2
> 
> To begin with, let's listen other's opinion whether they can fix it
> by other approach without calling __vmalloc with such flags.
> 
> So this patch adds warning in __vmalloc_node_range to detect it and
> to be fixed hopely. __vmalloc_node_range isn't random chocie because
> all caller which has gfp_mask of map_vm_area use it through __vmalloc_area_node.
> And __vmalloc_area_node is current static function and is called by only
> __vmalloc_node_range. So warning in __vmalloc_node_range would cover all
> vmalloc functions which have gfp_t argument.
>
> I Cced related maintainers.
> If I miss someone, please Cced them.
> 
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1648,6 +1648,10 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align,
>  	void *addr;
>  	unsigned long real_size = size;
>  
> +	WARN_ON_ONCE(!(gfp_mask & __GFP_WAIT) ||
> +			!(gfp_mask & __GFP_IO) ||
> +			!(gfp_mask & __GFP_FS));
> +
>  	size = PAGE_ALIGN(size);
>  	if (!size || (size >> PAGE_SHIFT) > totalram_pages)
>  		goto fail;

Well.  What are we actually doing here?  Causing the kernel to spew a
warning due to known-buggy callsites, so that users will report the
warnings, eventually goading maintainers into fixing their stuff.

This isn't very efficient :(

It would be better to fix that stuff first, then add the warning to
prevent reoccurrences.  Yes, maintainers are very naughty and probably
do need cattle prods^W^W warnings to motivate them to fix stuff, but we
should first make an effort to get these things fixed without
irritating and alarming our users.  

Where are these offending callsites?

  reply	other threads:[~2012-05-02 19:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-02  4:28 [PATCH] vmalloc: add warning in __vmalloc Minchan Kim
2012-05-02 19:46 ` Andrew Morton [this message]
2012-05-03  1:02   ` Minchan Kim
2012-05-03  5:46     ` Sage Weil
2012-05-03  6:30       ` Nick Piggin
2012-05-03  7:13         ` Artem Bityutskiy
2012-05-03  7:14           ` Nick Piggin
2012-05-03 13:48         ` Steven Whitehouse
2012-05-03  5:55   ` Artem Bityutskiy
2012-05-03 11:11   ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120502124610.175e099c.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=adrian.hunter@intel.com \
    --cc=davem@davemloft.net \
    --cc=dedekind1@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=jmorris@namei.org \
    --cc=kosaki.motohiro@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=neilb@suse.de \
    --cc=rientjes@google.com \
    --cc=sage@newdream.net \
    --cc=swhiteho@redhat.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).