All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: linux-kernel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] printk: fix recursion of logbuf_lock spinlock
Date: Fri, 25 Nov 2011 12:49:06 +0100	[thread overview]
Message-ID: <1322221746.2535.3.camel@laptop> (raw)
In-Reply-To: <1322161455-21242-1-git-send-email-aarcange@redhat.com>

On Thu, 2011-11-24 at 20:04 +0100, Andrea Arcangeli wrote:
>  kernel/printk.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 1455a0d..fc3c831 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -1259,9 +1259,9 @@ void console_unlock(void)
>  
>         console_may_schedule = 0;
>  
> -again:
>         for ( ; ; ) {
>                 raw_spin_lock_irqsave(&logbuf_lock, flags);
> +again:
>                 wake_klogd |= log_start - log_end;
>                 if (con_start == log_end)
>                         break;                  /* Nothing to print */ 

The better change would be the below one, since that avoid doing
console_trylock() while holding logbuf_lock. The problem is that
console_trylock() can do up() and thus result in a wakeup while holding
logbuf_lock and we're trying to get rid of locks nested under
logbug_lock.

---
Subject: printk: Avoid double lock acquire

Commit 4f2a8d3cf5e ("printk: Fix console_sem vs logbuf_lock unlock
race") introduced another silly bug where we would want to acquire an
already held lock. Avoid this.

Reported-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/printk.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index dfd8f73..187662f 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1292,10 +1292,11 @@ void console_unlock(void)
 	raw_spin_lock(&logbuf_lock);
 	if (con_start != log_end)
 		retry = 1;
+	raw_spin_unlock_irqrestore(&logbuf_lock, flags);
+
 	if (retry && console_trylock())
 		goto again;
 
-	raw_spin_unlock_irqrestore(&logbuf_lock, flags);
 	if (wake_klogd)
 		wake_up_klogd();
 }



      reply	other threads:[~2011-11-25 11:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-24 19:04 [PATCH] printk: fix recursion of logbuf_lock spinlock Andrea Arcangeli
2011-11-25 11:49 ` Peter Zijlstra [this message]

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=1322221746.2535.3.camel@laptop \
    --to=peterz@infradead.org \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@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 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.