Am Montag, 27. November 2006 23:08 schrieb Alan Stern:
> On Mon, 27 Nov 2006, Oliver Neukum wrote:
> > IMHO by a previous dropping all dentries, you make sure the usage count of
> > an inode will monotonically decrease.
>
> Sure. However suppose there used to be open file references and they have
> all been closed. The inode exists, but its usage count is 0 so it may be
> deallocated at any time. (I believe inodes remain for a while in some
> sort of cache even when they aren't in use.) Now the driver calls
> device_remove_file(). How does your new code in sysfs_drop_entry()
> prevent the inode from being deallocated while you are trying to call
> orphan_all_buffers()?
OK, let's make some basic observation. Sysfs exists in memory only,
as long as the dentry exists the inode cannot be freed. Therefore the
window for the race is only between dropping the dentry and orphaning
all buffers.
sysfs_release() will always hold a reference for the inode, so the inode
must be touched before the reference is dropped. How about:
static int sysfs_release(struct inode * inode, struct file * filp)
{
struct kobject * kobj = to_kobj(filp->f_dentry->d_parent);
struct attribute * attr = to_attr(filp->f_dentry);
struct module * owner = attr->owner;
struct sysfs_buffer * buffer = filp->private_data;
if (buffer)
remove_from_collection(buffer, inode);
if (kobj)
kobject_put(kobj);
/* After this point, attr should not be accessed. */
module_put(owner);
if (buffer) {
if (buffer->page)
free_page((unsigned long)buffer->page);
kfree(buffer);
}
return 0;
}
orphan_all_buffers() must be called with an additional reference to
the inode. So how about:
void sysfs_drop_dentry(struct sysfs_dirent * sd, struct dentry * parent)
{
struct dentry * dentry = sd->s_dentry;
struct inode * inode;
if (dentry) {
inode = dentry->d_inode;
igrab(inode); /* must work as dentry is still valid */
spin_lock(&dcache_lock);
spin_lock(&dentry->d_lock);
if (!(d_unhashed(dentry) && dentry->d_inode)) {
dget_locked(dentry);
__d_drop(dentry);
spin_unlock(&dentry->d_lock);
spin_unlock(&dcache_lock);
simple_unlink(parent->d_inode, dentry);
orphan_all_buffers(inode);
} else {
spin_unlock(&dentry->d_lock);
spin_unlock(&dcache_lock);
}
iput(inode);
}
}
Regards
Oliver
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
