public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Gabor Gombas <gombasg@sztaki.hu>,
	Dave Young <hidave.darkstar@gmail.com>,
	linux-kernel@vger.kernel.org, bluez-devel@lists.sourceforge.net,
	Greg KH <greg@kroah.com>,
	ebiederm@xmission.com
Subject: Re: [Bluez-devel] Oops involving RFCOMM and sysfs
Date: Sun, 06 Jan 2008 11:54:43 +0900	[thread overview]
Message-ID: <478042F3.7010002@gmail.com> (raw)
In-Reply-To: <20080106021826.GR27894@ZenIV.linux.org.uk>

Hello, Al Viro.

Al Viro wrote:
> On Sun, Jan 06, 2008 at 11:07:52AM +0900, Tejun Heo wrote:
> 
>> Right, I haven't thought about that.  When sysfs_get_dentry() is called,
>> @sd is always valid so unless there was existing negative dentry, lookup
>> is guaranteed to return positive dentry, but by populating dcache with
>> negative dentry before a node is created, things can go wrong.  I don't
>> think that's what's going on here tho.  If that was the case, the
>> while() loop looking up the next sd to lookup (@cur) should have blown
>> up as negative dentry will have NULL d_fsdata which doesn't match any sd.
> 
> No.  What happens if sd gets unlinked while we are on the way to
> sysfs_get_dentry() and so does its parent?

That means sysfs_remove_dir() is called on parent while other operations
are in progress on children, right?  sysfs has never allowed such things
&& AFAIK no one does that.  It's somewhat implied in the interface (such
as recursive removing) but I fully agree it's problematic.  Things like
these are why I think we need to unify/simplify locking as I wrote
previously.

> The parent is off the
> sibling list, we get negative dentry from lookup.  It's not hashed,
> so won't stick around in dcache (which is apparently what you are
> thinking about).  However, _THIS_ lookup has returned you a dentry
> with NULL ->d_inode and you are well and truly buggered.

So, the assumption is that while @sd is held and being operated on its
ancestry can never change except for rename and move and those are
protected with sysfs_rename_mutex.

I agree that the locking is awkward but sysfs's internal implementation
has completely changed over last six or so months and the conversion
still isn't complete.  It isn't pretty at all but is at least
understandable now, IMHO.  :-)

The last pending change is separating out kobject from sysfs and
simplifying locking.  I had some disagreement with Greg about the future
of kobject and then there was tsunami of ATA bugs because most major
distros converted to libata from IDE in the second half of the last year
and I'm still buried under it.  I'll get back to sysfs once these ATA
bugs subside a bit.

Thanks.

-- 
tejun

  reply	other threads:[~2008-01-06  2:55 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-28 17:32 Oops involving RFCOMM and sysfs Gabor Gombas
2007-12-29  8:07 ` [Bluez-devel] " Dave Young
2008-01-02 14:48   ` Gabor Gombas
2008-01-02 15:16   ` Gabor Gombas
2008-01-03 13:16     ` Gabor Gombas
2008-01-04  1:05       ` Dave Young
2008-01-07  8:07         ` Tejun Heo
2008-01-07 14:10         ` Gabor Gombas
2008-01-05  7:50     ` Al Viro
2008-01-05 14:30       ` Tejun Heo
2008-01-05 19:45         ` Al Viro
2008-01-06  2:07           ` Tejun Heo
2008-01-06  2:18             ` Al Viro
2008-01-06  2:54               ` Tejun Heo [this message]
2008-01-06  3:35                 ` Al Viro
2008-01-06  3:54                   ` Tejun Heo
2008-01-07  2:37             ` Tejun Heo
2008-01-07  8:21               ` Eric W. Biederman
2008-01-07  9:17                 ` Tejun Heo
2008-01-07  9:18                   ` Tejun Heo
2008-01-07  9:22                     ` Al Viro
2008-01-07 10:33                       ` Eric W. Biederman
2008-01-07 14:13       ` Gabor Gombas
2008-01-07 15:24         ` Tejun Heo
2008-01-07 21:00           ` Gabor Gombas
2008-01-08  9:42             ` Tejun Heo
2008-01-08 13:32               ` Gabor Gombas
2008-01-09  9:16                 ` Tejun Heo
2008-01-09 15:57                   ` Cornelia Huck
2008-01-10  1:11                   ` Dave Young
2008-01-11 23:09                     ` Gabor Gombas
2008-01-14  7:05                       ` Dave Young
2008-01-14 12:52                         ` Cornelia Huck
2008-01-15  1:57                           ` Dave Young
2008-01-16  1:02                             ` Dave Young
2008-01-16 23:06                               ` Gabor Gombas
2008-01-17  7:24                                 ` Dave Young
2008-01-17  8:15                                   ` Dave Young
2008-01-17 11:42                                     ` Cornelia Huck
2008-01-18  3:37                                       ` Dave Young
2008-01-18  9:19                                         ` Cornelia Huck
2008-01-18 10:23                                           ` Cornelia Huck
2008-01-18 10:34                                             ` Dave Young
2008-01-18 11:26                                               ` Cornelia Huck
2008-01-21  3:15                                                 ` Dave Young
2008-01-21 15:09                                                   ` [Patch] Driver core: Cleanup get_device_parent() in device_add() and device_move() Cornelia Huck
2008-01-10 10:15                   ` [Bluez-devel] Oops involving RFCOMM and sysfs Gabor Gombas

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=478042F3.7010002@gmail.com \
    --to=htejun@gmail.com \
    --cc=bluez-devel@lists.sourceforge.net \
    --cc=ebiederm@xmission.com \
    --cc=gombasg@sztaki.hu \
    --cc=greg@kroah.com \
    --cc=hidave.darkstar@gmail.com \
    --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