[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a781481a0707180949o2a3a89c9rb4c5f1b77e964cc7@mail.gmail.com>
Date: Wed, 18 Jul 2007 22:19:20 +0530
From: "Satyam Sharma" <satyam.sharma@...il.com>
To: "Tejun Heo" <htejun@...il.com>
Cc: "Gabriel C" <nix.or.die@...glemail.com>,
"Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
"Christoph Lameter" <clameter@....com>, gregkh@...e.de,
miles.lane@...il.com
Subject: Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure path
On 7/18/07, Tejun Heo <htejun@...il.com> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists