All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Boyer <jwboyer@redhat.com>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: 3.0-git15 Atomic scheduling in pidmap_init
Date: Thu, 4 Aug 2011 13:31:45 -0400	[thread overview]
Message-ID: <20110804173145.GM2096@zod.bos.redhat.com> (raw)
In-Reply-To: <20110804162658.GZ13065@linux.vnet.ibm.com>

On Thu, Aug 04, 2011 at 09:26:58AM -0700, Paul E. McKenney wrote:
> > > You really do need to be able to handle set_need_resched() at random
> > > times, and at first glance it appears to me that the warning could be
> > > triggered at runtime as well.  If so, the real fix is elsewhere, right?
> > > Especially given that the patch imposes extra cost at runtime...
> > 
> > In staring at it for a while it seems to be a combination of being in
> > atomic context according to the scheduler but things in early boot using
> > GFP_KERNEL.  At the point we're at in the boot, that is perfectly legal
> > as it's not being called from an interrupt handler and the mm subsystem
> > should be all setup, but we're still really early in boot and preempt is
> > disabled.
> 
> Isn't preemption disabled at that point in boot?  And isn't GFP_KERNEL
> illegal within preemption-disabled regions?

Yes, it's disabled.  I'm not sure if it's illegal or not.  pidmap_init
is called from start_kernel on line 598 of main.c.  local_irq_enable is
called on line 553, followed immediately by this comment:

	/* Interrupts are enabled now so all GFP allocations are safe. */
	gfp_allowed_mask = __GFP_BITS_MASK;

	kmem_cache_init_late();

So the comments there lead me to think I have no clue :).  That's mostly
why I'm asking for feedback here.  I don't have a huge amount of
experience in what is and isn't allowed in the early bootup path.

> >            As I mentioned before, KMEM_CACHE calls kmalloc with
> > GFP_KERNEL and I don't think we want to change that.
> > 
> > Once we're past early boot, I would expect that things running in true
> > atomic context won't be calling KMEM_CACHE or using GFP_KERNEL.  Or
> > maybe I hope?
> > 
> > I understand the desire to avoid another conditional, but I certainly
> > don't have any other suggestions at the moment.
> 
> How about doing GFP_ATOMIC on allocations done during that portion
> of the boot patch for which preemption is disabled?

Well, in the pidmap_init case there are two spots relevant to this.  The
first is the kzalloc call on line 562 of kernel/pid.c.  I could change
that to use GFP_IOFS even, and it avoids the backtrace from there. (And
I did that originally.)

However, the call on line 567 to KMEM_CACHE calls into
kmem_cache_create.  There is a flags variable, but it's for slab flags,
and kmem_cache_create calls kmalloc internally with GFP_KERNEL.  I don't
see kmem_cache_create_atomic or otherwise that would avoid this.  None
of that code is new either, most if it dating back to 2008.

The same issue exists in some of the next functions called in
start_kernel, like anon_vma_init, cred_init, fork_init, etc.  They all
call kmem_cache_create.

I could be missing something obvious, but I don't see a way to avoid
using GFP_KERNEL without a lot of rip-up in the rest of the init path.

josh

  reply	other threads:[~2011-08-04 17:32 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-01 15:46 3.0-git15 Atomic scheduling in pidmap_init Josh Boyer
2011-08-04 11:46 ` Josh Boyer
2011-08-04 14:04   ` Paul E. McKenney
2011-08-04 15:06     ` Josh Boyer
2011-08-04 16:26       ` Paul E. McKenney
2011-08-04 17:31         ` Josh Boyer [this message]
2011-08-05  1:19           ` Josh Boyer
2011-08-05  6:56             ` Paul E. McKenney
2011-08-05 14:22               ` Josh Boyer
2011-08-05 17:08                 ` Frederic Weisbecker
2011-08-05 22:26                   ` Paul E. McKenney
2011-08-05 23:12                     ` Frederic Weisbecker
2011-08-08  2:09                       ` Paul E. McKenney
2011-08-08  2:55                         ` Frederic Weisbecker
2011-08-08  3:10                           ` Paul E. McKenney
2011-08-09 11:35                             ` Frederic Weisbecker
2011-08-10 12:45                               ` Josh Boyer
2011-08-10 14:53                                 ` Frederic Weisbecker
2011-08-10 15:03                                   ` Josh Boyer
2011-08-14 23:04                                 ` Paul E. McKenney
2011-08-15 14:04                                   ` Josh Boyer
2011-08-15 15:20                                     ` Paul E. McKenney
2011-08-17 22:37                                       ` Josh Boyer
2011-08-17 22:49                                         ` Paul E. McKenney
2011-08-17 23:02                                           ` Josh Boyer
2011-08-17 23:06                                             ` Frederic Weisbecker
2011-08-17 23:17                                               ` Josh Boyer
2011-08-18 18:35                                                 ` Paul E. McKenney
2011-08-18 19:11                                                   ` Josh Boyer
2011-08-18 21:00                                                   ` Andrew Morton
2011-08-18 21:23                                                     ` Paul E. McKenney
2011-08-18 21:55                                                       ` Paul E. McKenney
2011-08-18 22:21                                                         ` Josh Boyer
2011-08-18 23:01                                                           ` Paul E. McKenney
2011-08-24 22:45                                                         ` Frederic Weisbecker
2011-08-24 23:12                                                           ` Paul E. McKenney
2011-08-24 23:34                                                             ` Frederic Weisbecker
2011-08-24 23:57                                                               ` Paul E. McKenney
2011-08-18 22:19                                                       ` Josh Boyer
2011-08-18 23:16                                                         ` Paul E. McKenney
2011-08-18 23:27                                                           ` Andrew Morton
2011-08-19  0:38                                                             ` Paul E. McKenney

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=20110804173145.GM2096@zod.bos.redhat.com \
    --to=jwboyer@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.