From: Linus Torvalds <torvalds@linux-foundation.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH][RFC] %pd - for printing dentry name
Date: Mon, 1 Feb 2010 14:37:32 -0800 (PST) [thread overview]
Message-ID: <alpine.LFD.2.00.1002011431490.3664@localhost.localdomain> (raw)
In-Reply-To: <20100201222511.GA12882@ZenIV.linux.org.uk>
On Mon, 1 Feb 2010, Al Viro wrote:
>
> I propose to add a new format - %pd. It would print dentry name.
> However, unlike everything else in vsnprintf, it would NOT be locking-agnostic.
> It would grab and release dentry->d_lock. And yes, I hate that as much as
> anyone. I don't see any sane alternative.
So I do hate it a lot too, and one of the main reasons I hate it is that
that format is then subtly but horribly broken inside sections that
already hold the lock.
And I can hear you say "What *subtle*? It's an instant deadlock!", but the
most likely reason for such a printk would be some debug statement etc
that would seldom/ever trigger - like a BUG_ON or whatever. And that "we
take the lock while printing" would thus become a rather nasty "machine
died silently" issue.
So I _really_ hate taking locks in the printk paths. We've had serious
problems with that in the past. It has made for some very annoying issues.
> * don't use %pd under dentry->d_lock, use dentry->d_name.name instead; in
> that case it *is* safe. Incidentally, ->d_lock isn't held a lot.
I realize we can just call it a rule, and yes, d_lock is held much less
than something like console_lock etc that we've had ABBA issues with, but
still..
> Comments?
Quite frankly, I'd _much_ rather see something like just always freeing
the dentry names (when they aren't inlined) using RCU. The VFS layer quite
possibly would want to do that anyway at some point (eg Nick's VFS
scalability patches), and then we could make it just a RCU read-lock or
whatever (interrupt disable, what-not) instead.
And I'm much happier with printk doing that kind of thing, and wouldn't
have issues with that kind of much weaker locking.
Linus
next prev parent reply other threads:[~2010-02-01 22:37 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-01 22:25 [PATCH][RFC] %pd - for printing dentry name Al Viro
2010-02-01 22:34 ` Al Viro
2010-02-01 22:37 ` Linus Torvalds [this message]
2010-02-01 23:18 ` Al Viro
2010-02-02 1:06 ` Al Viro
2010-02-02 5:55 ` Eric W. Biederman
2010-02-02 17:01 ` Al Viro
2010-02-02 18:10 ` Olivier Galibert
2010-02-02 19:19 ` Eric W. Biederman
2010-02-03 3:04 ` Al Viro
2010-02-04 4:53 ` Al Viro
2010-02-02 4:22 ` Linus Torvalds
2010-02-02 5:00 ` Al Viro
2010-02-02 6:36 ` Nick Piggin
2010-02-04 6:02 ` Al Viro
2010-02-04 7:40 ` Nick Piggin
2010-02-02 6:53 ` Paul E. McKenney
2010-02-02 7:09 ` Al Viro
2010-02-02 13:32 ` Matthew Wilcox
2010-02-02 15:56 ` Linus Torvalds
2010-02-02 16:13 ` Matthew Wilcox
2010-02-02 16:43 ` Al Viro
2010-02-03 10:52 ` Paul E. McKenney
2010-02-03 2:49 ` Paul E. McKenney
2010-02-04 15:29 ` Linus Torvalds
2010-02-04 16:02 ` Paul E. McKenney
2010-02-04 17:13 ` Linus Torvalds
2010-02-04 17:36 ` Al Viro
2010-02-07 16:34 ` Paul E. McKenney
2010-02-01 22:45 ` Joe Perches
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=alpine.LFD.2.00.1002011431490.3664@localhost.localdomain \
--to=torvalds@linux-foundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--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 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).