Re: [Bluez-devel] Oops involving RFCOMM and sysfs

[Tejun Heo <htejun@gmail.com>]

Hello,

Al Viro wrote:
> On Sun, Jan 06, 2008 at 11:54:43AM +0900, Tejun Heo wrote:
>> 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.
> 
> All it takes is kobject_rename() or kobject_move() called asynchronously 
> wrt removal...  I don't see an explicit ban for that.

There really hasn't been any stone-set rules for how sysfs can be used
and what operations are allowed simultaneously although sysfs
implementation always had a lot of assumptions about which ops can and
can't be done simultaneously.

The general assumption is that the caller is responsible for its and its
children's lifetime management which is usually followed as sysfs nodes
are removed when a device goes away or driver detaches at which point no
other ops should be in progress anyway.

I think this stems partially from tight coupling with kobject / driver
model.  kobject and driver model have certain rules about how they can
be used (again not explicit enough) and sysfs implementation kind of
piggy backed on those rules and added its own assumptions on top of it.
 After a while, it's hard to track what's assumed where and implementing
or changing one feature somewhere breaks some assumption elsewhere.
This is the primary reason why I think sysfs should be an independent
component which the driver model uses not something intertwined into
driver model while having mostly separate implementation.

> FWIW, what happens here *is* fishy, but I don't see an outright ban on
> that in documentation - rfcomm_tty_open() does
>                 device_move(dev->tty_dev, rfcomm_get_device(dev));
> when we get openers, rfcomm_tty_close() does
>                 device_move(dev->tty_dev, NULL);
> when the number of openers hits zero.  Can happen repeatedly.
> 
> Note that device_move() with new parent being NULL is explicitly allowed
> and handled, so...

(cc'ing Kay Sievers) IIRC, that's device class compatibility thing.
Overlapping move's are okay tho as they're protected from each other by
sysfs_rename_mutex.  I'll take a look at the rfcomm code and see what's
going on tomorrow.  Gotta hit the road now.

Thanks.

-- 
tejun