Re: [RFD driver-core] Lifetime problems of the current driver model

Greg KH
2007-04-02T09:33:24Z

On Mon, Apr 02, 2007 at 04:59:43AM +0900, Tejun Heo wrote:
>
> Dependencies between sysfs/kobject objects are clearer now and I think
> I got the locking and referencing correct. This patch immediately
> fixes 'sysfs attr grabbing the wrong kobject and module' problem -
> sysfs and module lifetimes are unrelated now. We can drop
> half-working attr->owner.
>
> * A sysfs node no longer hold reference to its kobject. It just
> attaches itself to the kobject on creation and detaches on deletion.
>
> * For a dir node, sysfs_dirent no longer directly points to kobject.
> It points to sysfs_dir which contains pointer to kobject and a rwsem
> to protect it.
>
> * An open file doesn't hold a reference to kobject. It holds a
> reference to sysfs_dirent. kobject pointer is verified and
> show/store are performed with rwsem read-locked. Deletion
> disconnects the sysfs from its kobject while the rwsem is
> write-locked. This mechanism replaces buffer orphaning and kobject
> validation during open.

Ah, very nice.

> * attr ops is determined on sysfs node creation not on open. This is
> a step toward making kobject opaque in sysfs.
>
> * bin files are handled similarly but mmap makes the open file hold
> reference to the kobject till it's closed. Any better ideas?

This is probably needed, and might be acceptable, and I think we can
live with it as there is only a very small number of sysfs files that
fall into this category.

> * symlink is reimplemented in terms of sysfs_dirents instead of
> kobjects. sysfs_dirent->s_parent is added and each sysfs_dirent
> holds reference to its parent. Currently walking up the tree
> requires read locking and unlocking sysfs_dir at each level. This
> can be removed if name is added to sysfs_dirent.
>
> * As kobject can be disconnected anytime and sysfs code still needs to
> look follow dentry link in kobject, kobject->dentry is protected by
> dcache_lock. Once kobject becomes opaque to sysfs, this hack can go
> away. All in all, making kobject completely opaque in sysfs isn't
> too far away after this patch although it would require mass code
> update.

What would need to be updated after this?

> What do you think about this approach? If it's acceptable, I'll test
> further and split the patch into logical steps to get it reviewed
> better.

At first glance, I think it looks fine, but Maneesh is the one who
understands this code better than anyone, so I would like to get his
opinion on it.

I think you should start splitting it all up into steps, which will help
us review it a whole lot easier, as overall, I think this is a very
worthy goal.

thanks so much for doing this work,

greg k-h