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

On 7/18/07, Tejun Heo <htejun@xxxxxxxxx> 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 ourselves (and in fact even allocate the new dirent for the
>>>> to-be-created symlink) and /then/ check for erroneous usage,
>>>> and then go about undoing all that we should never have done
>>>> at all. ] So this test could, and should, be made earlier, IMHO.
>>> Locking.
>> Well s/sysfs_find_dirent/sysfs_get_dirent/ then. And then simply put
>> down the reference later.
>
> Isn't that the current code?

Oops, somehow thought you were talking about allocating it first.
Gee... what difference does using sysfs_get_dirent() make? Do you think
the following code is correct?

sd = sysfs_get_dirent("some name");
if (sd != NULL)
return -EEXIST;
lock;
add_new_node("some name");
unlock;
sysfs_put_dirent(sd);

Nopes, it's not, of course. We'd need the parent's i_mutex as well
as the sysfs_mutex around both the EEXIST check as well as the
actual sysfs_add_one(), which is precisely what sysfs_addrm_start
and finish are, so you're right ... I'll factor this in.

Satyam
-
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/

Relevant Pages

  • cvs-src summary for June 7-14
    ... Here's the summary - hope you like locking. ... You can get old summaries, and an HTML version of this one, at ... Poul-Henning Kamp committed reference counting for the tty code; ...
    (freebsd-current)
  • [PATCH] vt: Make vt_pid a struct pid (making it pid wrap around safe).
    ... I took a good hard look at the locking and it appears the locking on ... vt_pid is the console semaphore. ... need to be careful because in the presence of an oops the console_sem ... except for a small window during a SAK, or oops handling. ...
    (Linux-Kernel)
  • Re: Study the TMJ/TMD cases involving attorneys
    ... Oops. ... The last line needed a reference. ... That sentence applies to the NIDCR and TIRR TMJD project at Univ. of ...
    (sci.med.dentistry)
  • Re: Type convertsion from string
    ... the second reference to each should have served as ... a back reference, but *1 was somehow to put the values, and *2 was ... It would have to be Reflection.Emit or dynamic compilation (compiler, ... I maintain that runtime properties are a very good way to ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: [linux-usb-devel] Re: [OOPS, usbcore, releaseintf] 2.6.0-test10-mm1
    ... > I didn't note the reason for the oops. ... My call to usb_put_dev in usbdev_release is releasing the kobject, ... which shows that the reference count was not already zero. ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)