linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Fix rq->lock vs logbuf_lock unlock race
@ 2013-02-18 12:53 Bu, Yitian
  2013-02-18 14:17 ` [tip:core/printk] printk: Fix rq-> lock vs logbuf_lock unlock lock inversion tip-bot for Bu, Yitian
  2013-02-20  8:45 ` [PATCH] Fix rq->lock vs logbuf_lock unlock race Peter Zijlstra
  0 siblings, 2 replies; 6+ messages in thread
From: Bu, Yitian @ 2013-02-18 12:53 UTC (permalink / raw)
  To: tglx, a.p.zijlstra; +Cc: linux-kernel, mingo

This patch is for kernel V3.7.9

>From 8796f4a2175a323aaa49ea8dd0fe68678dd5dccd Mon Sep 17 00:00:00 2001
From: ybu <ybu@qti.qualcomm.com>
Date: Mon, 18 Feb 2013 19:52:01 +0800
Subject: [PATCH] Fix rq->lock vs logbuf_lock unlock race

fix up the fallout from commit 07354eb1a74d1 ("locking printk:
Annotate logbuf_lock as raw")
Release console_sem after unlocking the logbuf_lock avoids some lock
inversion troubles between logbuf_lock and rq->lock.

Change-Id: I782c1d16e0e82bd156699f0f205f19781c4819e0
Signed-off-by: ybu <ybu@qti.qualcomm.com>
---
 kernel/printk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index f8e0b5a..3a49454 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1356,9 +1356,9 @@ static int console_trylock_for_printk(unsigned int cpu)
 		}
 	}
 	logbuf_cpu = UINT_MAX;
+	raw_spin_unlock(&logbuf_lock);
 	if (wake)
 		up(&console_sem);
-	raw_spin_unlock(&logbuf_lock);
 	return retval;
 }
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:core/printk] printk: Fix rq-> lock vs logbuf_lock unlock lock inversion
  2013-02-18 12:53 [PATCH] Fix rq->lock vs logbuf_lock unlock race Bu, Yitian
@ 2013-02-18 14:17 ` tip-bot for Bu, Yitian
  2013-02-20  8:45 ` [PATCH] Fix rq->lock vs logbuf_lock unlock race Peter Zijlstra
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Bu, Yitian @ 2013-02-18 14:17 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, a.p.zijlstra, ybu, tglx

Commit-ID:  dbda92d16f8655044e082930e4e9d244b87fde77
Gitweb:     http://git.kernel.org/tip/dbda92d16f8655044e082930e4e9d244b87fde77
Author:     Bu, Yitian <ybu@qti.qualcomm.com>
AuthorDate: Mon, 18 Feb 2013 12:53:37 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 18 Feb 2013 15:05:57 +0100

printk: Fix rq->lock vs logbuf_lock unlock lock inversion

commit 07354eb1a74d1 ("locking printk: Annotate logbuf_lock as raw")
reintroduced a lock inversion problem which was fixed in commit
0b5e1c5255 ("printk: Release console_sem after logbuf_lock"). This
happened probably when fixing up patch rejects.

Restore the ordering and unlock logbuf_lock before releasing
console_sem.

Signed-off-by: ybu <ybu@qti.qualcomm.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/E807E903FE6CBE4D95E420FBFCC273B827413C@nasanexd01h.na.qualcomm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 267ce78..e698e80 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1358,9 +1358,9 @@ static int console_trylock_for_printk(unsigned int cpu)
 		}
 	}
 	logbuf_cpu = UINT_MAX;
+	raw_spin_unlock(&logbuf_lock);
 	if (wake)
 		up(&console_sem);
-	raw_spin_unlock(&logbuf_lock);
 	return retval;
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix rq->lock vs logbuf_lock unlock race
  2013-02-18 12:53 [PATCH] Fix rq->lock vs logbuf_lock unlock race Bu, Yitian
  2013-02-18 14:17 ` [tip:core/printk] printk: Fix rq-> lock vs logbuf_lock unlock lock inversion tip-bot for Bu, Yitian
@ 2013-02-20  8:45 ` Peter Zijlstra
  2013-02-20  9:38   ` Bu, Yitian
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2013-02-20  8:45 UTC (permalink / raw)
  To: Bu, Yitian; +Cc: tglx, linux-kernel, mingo

On Mon, 2013-02-18 at 12:53 +0000, Bu, Yitian wrote:
> This patch is for kernel V3.7.9
> 
> From 8796f4a2175a323aaa49ea8dd0fe68678dd5dccd Mon Sep 17 00:00:00 2001
> From: ybu <ybu@qti.qualcomm.com>
> Date: Mon, 18 Feb 2013 19:52:01 +0800
> Subject: [PATCH] Fix rq->lock vs logbuf_lock unlock race
> 
> fix up the fallout from commit 07354eb1a74d1 ("locking printk:
> Annotate logbuf_lock as raw")
> Release console_sem after unlocking the logbuf_lock avoids some lock
> inversion troubles between logbuf_lock and rq->lock.

Please clarify how and where.. you're not actually supposed to use
printk() while holding rq->lock.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] Fix rq->lock vs logbuf_lock unlock race
  2013-02-20  8:45 ` [PATCH] Fix rq->lock vs logbuf_lock unlock race Peter Zijlstra
@ 2013-02-20  9:38   ` Bu, Yitian
  2013-02-20 10:19     ` Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Bu, Yitian @ 2013-02-20  9:38 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: tglx, linux-kernel, mingo

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1528 bytes --]



> -----Original Message-----
> From: Peter Zijlstra [mailto:a.p.zijlstra@chello.nl]
> Sent: Wednesday, February 20, 2013 4:45 PM
> To: Bu, Yitian
> Cc: tglx@linutronix.de; linux-kernel@vger.kernel.org; mingo@kernel.org
> Subject: Re: [PATCH] Fix rq->lock vs logbuf_lock unlock race
> 
> On Mon, 2013-02-18 at 12:53 +0000, Bu, Yitian wrote:
> > This patch is for kernel V3.7.9
> >
> > From 8796f4a2175a323aaa49ea8dd0fe68678dd5dccd Mon Sep 17 00:00:00
> 2001
> > From: ybu <ybu@qti.qualcomm.com>
> > Date: Mon, 18 Feb 2013 19:52:01 +0800
> > Subject: [PATCH] Fix rq->lock vs logbuf_lock unlock race
> >
> > fix up the fallout from commit 07354eb1a74d1 ("locking printk:
> > Annotate logbuf_lock as raw")
> > Release console_sem after unlocking the logbuf_lock avoids some lock
> > inversion troubles between logbuf_lock and rq->lock.
> 
> Please clarify how and where.. you're not actually supposed to use
> printk() while holding rq->lock.
> 

1. the patch 0b5e1c5255e, which is written by you,  release console_sem
after logbuf_lock. But the patch 07354eb1a revert one of your change, it 
reintroduced a lock inversion problem which was fixed in 0b5e1c5255.
The purpose of my patch is the same as your patch 0b5e1c5255e.

2. from printk comment: "This is printk(). It can be called from any context. 
We want it to work. ". I suppose to use printk in any context.


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix rq->lock vs logbuf_lock unlock race
  2013-02-20  9:38   ` Bu, Yitian
@ 2013-02-20 10:19     ` Peter Zijlstra
  2013-02-20 11:24       ` Bu, Yitian
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2013-02-20 10:19 UTC (permalink / raw)
  To: Bu, Yitian; +Cc: tglx, linux-kernel, mingo

On Wed, 2013-02-20 at 09:38 +0000, Bu, Yitian wrote:
> 
> 2. from printk comment: "This is printk(). It can be called from any
> context. 
> We want it to work. ". I suppose to use printk in any context.

Unfortunately that's not quite possible, rq->lock is really out of
bounds. At one point I tried 'fixing' this but there's a whole bunch of
nasty that's not going to go away.

I've since forgotten most of the details, but aside from logbuf problems
there's a whole host of issues with the console drivers themselves as
well.

If you really want to do this, use early_printk.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] Fix rq->lock vs logbuf_lock unlock race
  2013-02-20 10:19     ` Peter Zijlstra
@ 2013-02-20 11:24       ` Bu, Yitian
  0 siblings, 0 replies; 6+ messages in thread
From: Bu, Yitian @ 2013-02-20 11:24 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: tglx, linux-kernel, mingo

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1352 bytes --]

> Unfortunately that's not quite possible, rq->lock is really out of bounds. At
> one point I tried 'fixing' this but there's a whole bunch of nasty that's not
> going to go away.
> 
> I've since forgotten most of the details, but aside from logbuf problems
> there's a whole host of issues with the console drivers themselves as well.
> 
> If you really want to do this, use early_printk.

Hi Peter:

The patch  0b5e1c5255e("printk: Release console_sem after logbuf_lock" )
is as below:

			console_locked = 0;
-			up(&console_sem);
+			wake = 1;
 			retval = 0;
 		}
 	}
 	printk_cpu = UINT_MAX;
 	spin_unlock(&logbuf_lock);
+	if (wake)
+		up(&console_sem);
 	return retval;

the patch 07354eb1a74d1("locking, printk: Annotate logbuf_lock as raw ")
is as below:

	printk_cpu = UINT_MAX;
-	spin_unlock(&logbuf_lock);
 	if (wake)
 		up(&console_sem);
+	raw_spin_unlock(&logbuf_lock);
 	return retval;
 }

Note that the purpose of patch 07354eb1a74d1 is to use raw_spin_unlock
instead of spin_unlock, it is not supposed to change any lock sequences.

What I do is to revert this change to 0b5e1c5255e, and it is nothing to do with
how people use printk. It is just to recover a merge mistake.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-20 11:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-18 12:53 [PATCH] Fix rq->lock vs logbuf_lock unlock race Bu, Yitian
2013-02-18 14:17 ` [tip:core/printk] printk: Fix rq-> lock vs logbuf_lock unlock lock inversion tip-bot for Bu, Yitian
2013-02-20  8:45 ` [PATCH] Fix rq->lock vs logbuf_lock unlock race Peter Zijlstra
2013-02-20  9:38   ` Bu, Yitian
2013-02-20 10:19     ` Peter Zijlstra
2013-02-20 11:24       ` Bu, Yitian

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).