Re: [PATCH][RFC] %pd - for printing dentry name

[Linus Torvalds]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/