Re: [PATCH 1/5] sysfs: relay channel buffers as sysfs attributes

[Maneesh Soni]

Quoting Paul Mundt <lethal@xxxxxxxxxxxx>:

> On Thu, Feb 23, 2006 at 04:29:35PM +0530, Maneesh Soni wrote:
> > On Sun, Feb 19, 2006 at 11:07:57PM +0200, Paul Mundt wrote:
> > > +	dentry = lookup_one_len(filename, parent, strlen(filename));
> >
> > lookup_one_len() needs parent's i_mutex.
> Good catch, thanks.
>
> > > +	if (IS_ERR(dentry))
> > > +		sysfs_hash_and_remove(parent, filename);
> >
> > Also wondering if you have considered the case of -EEXIST?
> How is that going to happen? The line before we do sysfs_add_file(), and
> if that errors out, then we never make it to lookup_one_len().

well, there is no check in sysfs_add_file() (probably it should have?) 
for -EEXIST. Till
now, the dentry/inode for sysfs files are allocated only when
the file is looked up at the first instance. Where here (for relay files)
dentry/inodes are being allocated at the time of creation.

IOW, the check for existing files is being done at the time of
assigning the inode (as in fs/sysfs/inode.c:sysfs_create()) when 
someone looks for the
file.
->sysfs_lookup()-->sysfs_attach_attr()-->sysfs_create()

> The IS_ERR() check is pretty superfluous anyways, perhaps it makes more
> sense just to remove it and the sysfs_hash_and_remove() reference
> entirely.
hmm.. probably not, looks like it is needed. The extra sysfs_dirent which
got added in sysfs_add_file() will be removed by sysfs_hash_and_remove() in
case of -EEXIST.

Thanks
Maneesh

PS: Sorry for previous message with incorrect reply-to address.
-
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/