Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Miles Lane
On 7/18/07, Satyam Sharma <[EMAIL PROTECTED]> wrote: On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: > Tejun Heo wrote: > > Satyam Sharma wrote: > sysfs_find_dirent() -- to check for -EEXIST -- should be called > *before* we create the new dentry for the to-be-created symlink >

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: Tejun Heo wrote: > Satyam Sharma wrote: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the first place. [ It's weird to grab a reference on the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Tejun Heo wrote: > Satyam Sharma wrote: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the first place. [ It's weird to grab a reference on the target for ourselves (and in fact even allocate

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: Satyam Sharma wrote: > Readability, fewer LOC, 308 lesser bytes in kernel image and > faster for the common case -- not good enough for you?! Oh, well. Sorry, not agreed on readability. The rest doesn't really matter too much and please stop

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: >> > sysfs_find_dirent() -- to check for -EEXIST -- should be called >> > *before* we create the new dentry for the to-be-created symlink >> > in the first place. [ It's weird to grab a reference on the target >> > for ourselves (and in fact even allocate the new dirent for

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: >> On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: >> > Satyam Sharma wrote: >> > > On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: >> > >> There is a subtle bug in sysfs_create_link() failure path. When >> > >> symlink creation fails because there's already a node with the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: Satyam Sharma wrote: > A trivial nit: > > The cleanup ignores the return of sysfs_addrm_finish() -- functions > such as those could and should be void-returning. It doesn't even > need to return an int for success / failure ... I went over it's

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: Satyam Sharma wrote: >> Well, I dunno. Probably my taste just sucks. Please feel free to >> submit patches and/or suggest better ideas. > > OK, for example: > > sysfs_find_dirent() -- to check for -EEXIST -- should be called > *before* we create

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: >> sysfs_find_dirent() -- to check for -EEXIST -- should be called >> *before* we create the new dentry for the to-be-created symlink >> in the first place. [ It's weird to grab a reference on the target >> for ourselves (and in fact even allocate the new dirent for the >>

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: >> Well, I dunno. Probably my taste just sucks. Please feel free to >> submit patches and/or suggest better ideas. > > OK, for example: > > sysfs_find_dirent() -- to check for -EEXIST -- should be called > *before* we create the new dentry for the to-be-created symlink >

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Satyam Sharma <[EMAIL PROTECTED]> wrote: Hi, On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: > Satyam Sharma wrote: > > On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: > >> There is a subtle bug in sysfs_create_link() failure path. When > >> symlink creation fails because there's

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
Hi, On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: Satyam Sharma wrote: > On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: >> There is a subtle bug in sysfs_create_link() failure path. When >> symlink creation fails because there's already a node with the same >> name, the target

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: > On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: >> There is a subtle bug in sysfs_create_link() failure path. When >> symlink creation fails because there's already a node with the same >> name, the target sysfs_dirent is put twice - once by failure path of >>

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
Hi Tejun, Thanks for tracking this down and fixing it. On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Miles Lane
On 7/18/07, Tejun Heo <[EMAIL PROTECTED]> wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by failure path of sysfs_create_link() and once more when the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Cornelia Huck
On Wed, 18 Jul 2007 16:14:45 +0900, Tejun Heo <[EMAIL PROTECTED]> wrote: > There is a subtle bug in sysfs_create_link() failure path. When > symlink creation fails because there's already a node with the same > name, the target sysfs_dirent is put twice - once by failure path of >

[PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by failure path of sysfs_create_link() and once more when the symlink is released. Fix it by making only the

[PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by failure path of sysfs_create_link() and once more when the symlink is released. Fix it by making only the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Cornelia Huck
On Wed, 18 Jul 2007 16:14:45 +0900, Tejun Heo [EMAIL PROTECTED] wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by failure path of sysfs_create_link()

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Miles Lane
On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by failure path of sysfs_create_link() and once more when the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
Hi Tejun, Thanks for tracking this down and fixing it. On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put twice - once by failure path of sysfs_create_link() and

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
Hi, On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Satyam Sharma wrote: On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the target sysfs_dirent is put

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Satyam Sharma [EMAIL PROTECTED] wrote: Hi, On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Satyam Sharma wrote: On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: Well, I dunno. Probably my taste just sucks. Please feel free to submit patches and/or suggest better ideas. OK, for example: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the first place. [ It's weird to grab a reference on the target for ourselves (and in fact even allocate the new dirent for the to-be-created

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Satyam Sharma wrote: Well, I dunno. Probably my taste just sucks. Please feel free to submit patches and/or suggest better ideas. OK, for example: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Satyam Sharma wrote: A trivial nit: The cleanup ignores the return of sysfs_addrm_finish() -- functions such as those could and should be void-returning. It doesn't even need to return an int for success / failure ... I went over it's code,

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Satyam Sharma wrote: On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: There is a subtle bug in sysfs_create_link() failure path. When symlink creation fails because there's already a node with the same name, the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Satyam Sharma wrote: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the first place. [ It's weird to grab a reference on the target for ourselves (and in fact even allocate the new dirent for the

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Satyam Sharma wrote: Readability, fewer LOC, 308 lesser bytes in kernel image and faster for the common case -- not good enough for you?! Oh, well. Sorry, not agreed on readability. The rest doesn't really matter too much and please stop making

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Tejun Heo
Tejun Heo wrote: Satyam Sharma wrote: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the first place. [ It's weird to grab a reference on the target for ourselves (and in fact even allocate the new dirent for

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Satyam Sharma
On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Tejun Heo wrote: Satyam Sharma wrote: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the first place. [ It's weird to grab a reference on the target for

Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path

2007-07-18 Thread Miles Lane
On 7/18/07, Satyam Sharma [EMAIL PROTECTED] wrote: On 7/18/07, Tejun Heo [EMAIL PROTECTED] wrote: Tejun Heo wrote: Satyam Sharma wrote: sysfs_find_dirent() -- to check for -EEXIST -- should be called *before* we create the new dentry for the to-be-created symlink in the first place. [