linux-kernel - Re: [RFC] propagate gfp_t to page table alloc functions

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <20120424172554.c9c330dd.akpm@linux-foundation.org>
Date:	Tue, 24 Apr 2012 17:25:54 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Nick Piggin <npiggin@...il.com>,
	Minchan Kim <minchan@...nel.org>,
	Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
	Hugh Dickins <hughd@...gle.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Rik van Riel <riel@...hat.com>, Mel Gorman <mgorman@...e.de>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [RFC] propagate gfp_t to page table alloc functions

On Wed, 25 Apr 2012 09:05:12 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> wrote:

> (2012/04/25 8:55), KOSAKI Motohiro wrote:
> 
> > On Tue, Apr 24, 2012 at 7:49 PM, KAMEZAWA Hiroyuki
> > <kamezawa.hiroyu@...fujitsu.com> wrote:
> >> (2012/04/25 6:30), Andrew Morton wrote:
> >>
> >>> On Tue, 24 Apr 2012 17:48:29 +1000
> >>> Nick Piggin <npiggin@...il.com> wrote:
> >>>
> >>>>> Hmm, there are several places to use GFP_NOIO and GFP_NOFS even, GFP_ATOMIC.
> >>>>> I believe it's not trivial now.
> >>>>
> >>>> They're all buggy then. Unfortunately not through any real fault of their own.
> >>>
> >>> There are gruesome problems in block/blk-throttle.c (thread "mempool,
> >>> percpu, blkcg: fix percpu stat allocation and remove stats_lock").  It
> >>> wants to do an alloc_percpu()->vmalloc() from the IO submission path,
> >>> under GFP_NOIO.
> >>>
> >>> Changing vmalloc() to take a gfp_t does make lots of sense, although I
> >>> worry a bit about making vmalloc() easier to use!
> >>>
> >>> I do wonder whether the whole scheme of explicitly passing a gfp_t was
> >>> a mistake and that the allocation context should be part of the task
> >>> context.  ie: pass the allocation mode via *current.
> >>
> >> yes...that's very interesting.
> > 
> > I think GFP_ATOMIC is used non task context too. ;-)
> 
> Hmm, in interrupt context or some ? Can't we detect it ?

There are lots of practical details and I haven't begun to think it
through, mainly because it Isn't Going To Happen!

For example how do we handle spin_lock()?  Does spin_lock() now do

gfp_t spin_lock_2(spinlock_t *lock)
{
	gfp_t old_gfp = set_current_gfp(GFP_ATOMIC);
	spin_lock(lock);
	return old_gfp;
}

void spin_unlock_2(spinlock_t *lock, gfp_t old_gfp)
{
	spin_unlock(lock);
	set_current_gfp(old_gfp);
}

Well that's bad.  Currently we require programmers to keep track of
what context they're running in.  So they think about what they're
doing.  If we made it this easy, we'd see a big proliferation of
GFP_ATOMIC allocations, which is bad.

Requiring the spin_lock() caller to run set_current_gfp() would have
the same effect.



Or do we instead do this:

-	some_function(foo, bar, GFP_NOIO);
+	old_gfp = set_current_gfp(GFP_NOIO);
+	some_function(foo, bar);
+	set_current_gfp(old_gfp);

So the rule is "if the code was using an explicit GFP_foo then convert
it to use set_current_gfp().  If the code was receiving a gfp_t
variable from the caller then delete that arg".

Or something like that.  It's all too hopelessly impractical to bother
discussing - 20 years too late!


otoh, maybe a constrained version of this could be used to address the
vmalloc() problem alone.


otoh2, I didn't *want* blk-throttle.c to use GFP_NOIO for vmalloc(). 
GFP_NOIO is weak, unreliable and lame.  blk-throttle should find a way
of using GFP_KERNEL!
--
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/