netdev - Re: [PATCH 04/16] mm: allow PF_MEMALLOC from softirq context

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: <20120709165710.GC3515@breakpoint.cc>
Date:	Mon, 9 Jul 2012 18:57:10 +0200
From:	Sebastian Andrzej Siewior <sebastian@...akpoint.cc>
To:	Mel Gorman <mgorman@...e.de>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linux-MM <linux-mm@...ck.org>,
	Linux-Netdev <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	David Miller <davem@...emloft.net>, Neil Brown <neilb@...e.de>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Mike Christie <michaelc@...wisc.edu>,
	Eric B Munson <emunson@...bm.net>,
	Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [PATCH 04/16] mm: allow PF_MEMALLOC from softirq context

On Mon, Jul 09, 2012 at 11:04:42AM +0100, Mel Gorman wrote:
> > - lets assume your allocation happens with kmalloc() without __GFP_MEMALLOC
> >   and current->flags has PF_MEMALLOC ORed and your SLAB pool is empty. This
> >   forces SLAB to allocate more pages from the buddy allocator with it will
> >   receive more likely (due to ->current->flags + PF_MEMALLOC) but SLAB will
> >   drop this extra memory because the page has ->pf_memory (or something like
> >   that) set and the GFP_FLAGS do not have __GFP_MEMALLOC set.
> > 
> 
> It's recorded if the slab page was allocated from PFMEMALLOC reserves (see
> patch 2 from the swap over NBD series). slab will use this page for objects
> but only allocate them to callers that pass a gfp_pfmemalloc_allowed() check.
> kmalloc() users with either __GFP_MEMALLOC or PF_MEMALLOC will get
> the pages they need but they will not "leak" to !_GFP_MEMALLOC users as
> that would potentially deadlock.

Argh, I missed that gfp_to_alloc_flags() is not only called from
within the buddy allocater but also from slab. So this is fine then :)

One thing:
You only get current->flags |= PF_MEMALLOC in softirq _if_ the skb, which is 
passed to netif_receive_skb(), was allocated with __GFP_MEMALLOC. That
means if the NIC's RX allocation did not require an allocation from the
emergency pool (without ->pfmemalloc set) then you never use this extra
pool, even if this skb would end up in your swap socket. Also, the other way
around, where you allocate it from the emergency pool but it is a user
socket and you could drop it.

What about extending sk_set_memalloc() to record socket's ips + ports
in a separate list so that skb_pfmemalloc_protocol() might use that
information and decide on per-protocol basis if the skb is worth to
spend more ressource to deliver it. That means you would enable the
extra pool if the currently received skb is part of your swap socket and
not if the skb was allocated from the emergency pool.

That said, there is nothing wrong with the code as of now and this
optimization could be added later (if at all).

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html