From: "Américo Wang" <xiyou.wangcong@gmail.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: "Américo Wang" <xiyou.wangcong@gmail.com>,
"KOSAKI Motohiro" <kosaki.motohiro@jp.fujitsu.com>,
"Al Viro" <viro@zeniv.linux.org.uk>,
"Tavis Ormandy" <taviso@google.com>,
"Jeff Dike" <jdike@addtoit.com>, "Julien Tinnes" <jln@google.com>,
"Matt Mackall" <mpm@selenic.com>,
LKML <linux-kernel@vger.kernel.org>,
"Oleg Nesterov" <oleg@redhat.com>,
"Alan Cox" <alan@lxorguk.ukuu.org.uk>
Subject: [Patch] fix the lockdep warning in tty_fasync()
Date: Tue, 26 Jan 2010 23:58:19 +0800 [thread overview]
Message-ID: <20100126155819.GB3780@hack> (raw)
In-Reply-To: <m1ljflxcyl.fsf@fess.ebiederm.org>
On Tue, Jan 26, 2010 at 04:33:38AM -0800, Eric W. Biederman wrote:
>>>
>>> That or tweak __f_setown to use irqsave/irqrestore variants for it's
>>> locks, __f_setown is already atomic. I prefer that direction because the
>>> code is just a little simpler.
>>>
>>
>> Oh, very good advice!
>>
>> Patch is below.
>>
>> -------------->
>> Commit 703625118 causes a lockdep warning:
>>
>> [ INFO: possible irq lock inversion dependency detected ]
>> 2.6.33-rc5 #77
>> ---------------------------------------------------------
>> emacs/1609 just changed the state of lock:
>> (&(&tty->ctrl_lock)->rlock){+.....}, at: [<ffffffff8127c648>]
>> tty_fasync+0xe8/0x190
>> but this lock took another, HARDIRQ-unsafe lock in the past:
>> (&(&sighand->siglock)->rlock){-.....}
>>
>> This is due to we use write_lock_irq() in __f_setown() which turns
>> the IRQ on in write_unlock_irq(), causes this warning.
>>
>> Switch it ot write_lock_irqsave() and write_unlock_irqrestore(),
>> as suggested by Eric.
>>
>> Reported-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>> Signed-off-by: WANG Cong <xiyou.wangcong@gmail.com>
>>
>> ----
>>
>> diff --git a/fs/fcntl.c b/fs/fcntl.c
>> index 97e01dc..556b404 100644
>> --- a/fs/fcntl.c
>> +++ b/fs/fcntl.c
>> @@ -199,7 +199,8 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
>> static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
>> int force)
>> {
>> - write_lock_irq(&filp->f_owner.lock);
>> + int flags;
>
>Minor nit. This should be "unsigned long flags;"
>
Right... Below is an updated version.
Thanks.
------------>
Commit 703625118 causes a lockdep warning:
[ INFO: possible irq lock inversion dependency detected ]
2.6.33-rc5 #77
---------------------------------------------------------
emacs/1609 just changed the state of lock:
(&(&tty->ctrl_lock)->rlock){+.....}, at: [<ffffffff8127c648>]
tty_fasync+0xe8/0x190
but this lock took another, HARDIRQ-unsafe lock in the past:
(&(&sighand->siglock)->rlock){-.....}
This is due to we use write_lock_irq() in __f_setown() which turns
the IRQ on in write_unlock_irq(), causes this warning.
Switch it to write_lock_irqsave() and write_unlock_irqrestore(),
as suggested by Eric.
Reported-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Signed-off-by: WANG Cong <xiyou.wangcong@gmail.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
---
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 97e01dc..82cc8a7 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -199,7 +199,8 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
int force)
{
- write_lock_irq(&filp->f_owner.lock);
+ unsigned long flags;
+ write_lock_irqsave(&filp->f_owner.lock, flags);
if (force || !filp->f_owner.pid) {
put_pid(filp->f_owner.pid);
filp->f_owner.pid = get_pid(pid);
@@ -211,7 +212,7 @@ static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
filp->f_owner.euid = cred->euid;
}
}
- write_unlock_irq(&filp->f_owner.lock);
+ write_unlock_irqrestore(&filp->f_owner.lock, flags);
}
int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
next prev parent reply other threads:[~2010-01-26 15:56 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-26 3:20 [2.6.33-rc5] starting emacs makes lockdep warning KOSAKI Motohiro
2010-01-26 5:25 ` Américo Wang
2010-01-26 5:37 ` KOSAKI Motohiro
2010-01-26 5:49 ` KOSAKI Motohiro
2010-01-26 6:01 ` Américo Wang
2010-01-26 6:07 ` Al Viro
2010-01-26 6:24 ` Américo Wang
2010-01-26 6:54 ` Al Viro
2010-01-26 7:45 ` KOSAKI Motohiro
2010-01-26 8:45 ` Américo Wang
2010-01-26 9:14 ` Eric W. Biederman
2010-01-26 9:32 ` Américo Wang
2010-01-26 12:33 ` Eric W. Biederman
2010-01-26 15:58 ` Américo Wang [this message]
2010-01-27 1:09 ` [Patch] fix the lockdep warning in tty_fasync() KOSAKI Motohiro
2010-01-27 1:47 ` Greg KH
2010-01-26 6:17 ` [2.6.33-rc5] starting emacs makes lockdep warning Eric W. Biederman
2010-01-26 18:16 ` check_usage_backwards() && forwards? (Was: [2.6.33-rc5] starting emacs makes lockdep warning) Oleg Nesterov
2010-01-26 18:47 ` Peter Zijlstra
2010-01-27 2:58 ` Américo Wang
2010-01-27 13:15 ` [tip:core/urgent] lockdep: Fix check_usage_backwards() error message tip-bot for Oleg Nesterov
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=20100126155819.GB3780@hack \
--to=xiyou.wangcong@gmail.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=ebiederm@xmission.com \
--cc=jdike@addtoit.com \
--cc=jln@google.com \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpm@selenic.com \
--cc=oleg@redhat.com \
--cc=taviso@google.com \
--cc=viro@zeniv.linux.org.uk \
/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.