netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: David Miller <davem@davemloft.net>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	neilb@suse.de, a.p.zijlstra@chello.nl, michaelc@cs.wisc.edu,
	emunson@mgebm.net
Subject: Re: [PATCH 05/17] mm: allow PF_MEMALLOC from softirq context
Date: Mon, 14 May 2012 11:02:29 +0100	[thread overview]
Message-ID: <20120514100229.GA29102@suse.de> (raw)
In-Reply-To: <20120511.003951.1470088131186301605.davem@davemloft.net>

On Fri, May 11, 2012 at 12:39:51AM -0400, David Miller wrote:
> From: Mel Gorman <mgorman@suse.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@chello.nl>
> > Signed-off-by: Mel Gorman <mgorman@suse.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, 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>

  reply	other threads:[~2012-05-14 10:02 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-10 13:44 [PATCH 00/17] Swap-over-NBD without deadlocking V10 Mel Gorman
2012-05-10 13:44 ` [PATCH 01/17] mm: Serialize access to min_free_kbytes Mel Gorman
2012-05-10 13:44 ` [PATCH 02/17] mm: sl[au]b: Add knowledge of PFMEMALLOC reserve pages Mel Gorman
2012-05-10 13:44 ` [PATCH 03/17] mm: slub: Optimise the SLUB fast path to avoid pfmemalloc checks Mel Gorman
2012-05-10 13:44 ` [PATCH 04/17] mm: Introduce __GFP_MEMALLOC to allow access to emergency reserves Mel Gorman
2012-05-10 13:44 ` [PATCH 05/17] mm: allow PF_MEMALLOC from softirq context Mel Gorman
2012-05-11  4:39   ` David Miller
2012-05-14 10:02     ` Mel Gorman [this message]
2012-05-15 13:07       ` Mel Gorman
2012-05-10 13:44 ` [PATCH 06/17] mm: Only set page->pfmemalloc when ALLOC_NO_WATERMARKS was used Mel Gorman
2012-05-10 13:45 ` [PATCH 07/17] mm: Ignore mempolicies when using ALLOC_NO_WATERMARK Mel Gorman
2012-05-10 13:45 ` [PATCH 08/17] net: Introduce sk_allocation() to allow addition of GFP flags depending on the individual socket Mel Gorman
2012-05-11  4:49   ` David Miller
2012-05-11 14:12     ` Mel Gorman
2012-05-10 13:45 ` [PATCH 09/17] netvm: Allow the use of __GFP_MEMALLOC by specific sockets Mel Gorman
2012-05-11  4:50   ` David Miller
2012-05-10 13:45 ` [PATCH 10/17] netvm: Allow skb allocation to use PFMEMALLOC reserves Mel Gorman
2012-05-11  4:57   ` David Miller
2012-05-11 14:32     ` Mel Gorman
2012-05-11 14:42       ` Peter Zijlstra
2012-05-11 15:07         ` Mel Gorman
2012-05-11 21:17       ` David Miller
2012-05-10 13:45 ` [PATCH 11/17] netvm: Propagate page->pfmemalloc to skb Mel Gorman
2012-05-10 13:45 ` [PATCH 12/17] netvm: Propagate page->pfmemalloc from netdev_alloc_page " Mel Gorman
2012-05-11  5:01   ` David Miller
2012-05-11 14:46     ` Mel Gorman
2012-05-10 13:45 ` [PATCH 13/17] netvm: Set PF_MEMALLOC as appropriate during SKB processing Mel Gorman
2012-05-11  5:03   ` David Miller
2012-05-10 13:45 ` [PATCH 14/17] mm: Micro-optimise slab to avoid a function call Mel Gorman
2012-05-10 13:45 ` [PATCH 15/17] nbd: Set SOCK_MEMALLOC for access to PFMEMALLOC reserves Mel Gorman
2012-05-10 13:45 ` [PATCH 16/17] mm: Throttle direct reclaimers if PF_MEMALLOC reserves are low and swap is backed by network storage Mel Gorman
2012-05-10 13:45 ` [PATCH 17/17] mm: Account for the number of times direct reclaimers get throttled Mel Gorman
2012-05-10 17:17 ` [PATCH 00/17] Swap-over-NBD without deadlocking V10 Mike Christie
2012-05-11  5:04 ` David Miller
2012-05-11 15:45   ` Mel Gorman
2012-05-11 21:23     ` David Miller
2012-05-11 21:29       ` Andrew Morton
2012-05-14 11:10         ` Mel Gorman
  -- strict thread matches above, loose matches on Subject: below --
2012-05-17 14:50 [PATCH 00/17] Swap-over-NBD without deadlocking V11 Mel Gorman
2012-05-17 14:50 ` [PATCH 05/17] mm: allow PF_MEMALLOC from softirq context Mel Gorman

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=20120514100229.GA29102@suse.de \
    --to=mgorman@suse.de \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=emunson@mgebm.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=neilb@suse.de \
    --cc=netdev@vger.kernel.org \
    /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).