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: Threads - why isnt a whole object locked when ...?
    ... All of your questions are answerable by pointing out that you have an incorrect mental model of what it means to "lock" something. ... That is, while it's entirely sensible to people who are familiar with standard thread synchronization techniques, in reality it's not actually locking anything. ... Instead, it's using the reference as a sort of traffic signal for other threads, which are cooperating, to respect. ...
    (microsoft.public.dotnet.languages.csharp)
  • 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)