netdev - Re: [PATCH 05/17] 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: <20120514100229.GA29102@suse.de>
Date:	Mon, 14 May 2012 11:02:29 +0100
From:	Mel Gorman <mgorman@...e.de>
To:	David Miller <davem@...emloft.net>
Cc:	akpm@...ux-foundation.org, linux-mm@...ck.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	neilb@...e.de, a.p.zijlstra@...llo.nl, michaelc@...wisc.edu,
	emunson@...bm.net
Subject: Re: [PATCH 05/17] mm: allow PF_MEMALLOC from softirq context

On Fri, May 11, 2012 at 12:39:51AM -0400, David Miller wrote:
> From: Mel Gorman <mgorman@...e.de>
> Date: Thu, 10 May 2012 14:44:58 +0100
> 
> > This is needed to allow network softirq packet processing to make
> > use of PF_MEMALLOC.
> > 
> > Currently softirq context cannot use PF_MEMALLOC due to it not being
> > associated with a task, and therefore not having task flags to fiddle
> > with - thus the gfp to alloc flag mapping ignores the task flags when
> > in interrupts (hard or soft) context.
> > 
> > Allowing softirqs to make use of PF_MEMALLOC therefore requires some
> > trickery.  We basically borrow the task flags from whatever process
> > happens to be preempted by the softirq.
> > 
> > So we modify the gfp to alloc flags mapping to not exclude task flags
> > in softirq context, and modify the softirq code to save, clear and
> > restore the PF_MEMALLOC flag.
> > 
> > The save and clear, ensures the preempted task's PF_MEMALLOC flag
> > doesn't leak into the softirq. The restore ensures a softirq's
> > PF_MEMALLOC flag cannot leak back into the preempted process.
> > 
> > Signed-off-by: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> > Signed-off-by: Mel Gorman <mgorman@...e.de>
> 
> We're now making changes to task->flags from both base and
> softirq context, but with non-atomic operations and no other
> kind of synchronization.
> 
> As far as I can tell, this has to be racy.
> 

I'm not seeing the race you are thinking of.

Softirqs can run on multiple CPUs sure but the same task should not be
	executing the same softirq code. Interrupts are disabled and the
	executing process cannot sleep in softirq context so the task flags
	cannot "leak" nor can they be concurrently modified.

Softirqs are not execued from hard interrupt context so there are no
	races with hardirqs.

If the softirq is deferred to ksoftirq then its flags may be used
	instead of a normal tasks but as the softirq cannot be preempted,
	the PF_MEMALLOC flag does not leak to other code by accident.

When __do_softirq() is finished, care is taken to restore the
	PF_MEMALLOC flag to the value when __do_softirq() started. They
	should not be accidentally clearing the flag.

I'm not seeing how current->flags can be modified while the softirq handler
is running in such a way that information is lost or misused. There
would be a problem if softirqs used GFP_KERNEL because the presense of
the PF_MEMALLOC flag would prevent the use of direct reclaim but softirqs
cannot use direct reclaim anyway.

> If this works via some magic combination of invariants, you
> absolutely have to document this, verbosely.

Did I miss a race you are thinking of or should I just add the above
explanation to the changelog?

-- 
Mel Gorman
SUSE Labs
--
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