linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

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