From: Tejun Heo <tj@kernel.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
kay@vrfy.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHSET] sysfs: implement sysfs_remove()
Date: Fri, 27 Sep 2013 09:49:24 -0400 [thread overview]
Message-ID: <20130927134924.GC3205@htj.dyndns.org> (raw)
In-Reply-To: <20130926234433.GA12549@kroah.com>
Hey,
On Thu, Sep 26, 2013 at 04:44:33PM -0700, Greg KH wrote:
> I seem to remember some issues here as well, probably with scsi devices,
> that kept us from doing this in this manner. Can you test this on
> removing some scsi devices and see if everything still works properly
> with this patchset applied?
>From kobject side, it shouldn't really change as kobj holds an extra
ref on the associated sysfs_dirent. While sysfs_dirents are
recursively unlinked, they'll still be there until the associated kobj
is released. And, yeap, SCSI works fine.
> I'm really hesitant to apply this series, as it does change how sysfs
> works in this area, why do you need these changes?
Because the interface we now have is broken and I don't want spread
that to new users. Our current semantics is somewhere between "rmdir"
and "rm -rf". We remove files immediately below a directory but don't
recurse.
I think this was designed this way because of the tight coupling with
kobj. As a directory always used to be represented by a kobj and a
kobj's lifetime is managed separately, chaining directory removal to
kobj lifetime was good enough; however, please note that this is no
longer true with the sysfs group, so now sysfs is in a weird place
where it doesn't require attribute removals right below the kobj
directory but does require group removals. Even that is inconsistent
as group can be embedded in the parent kobj directory, and I'm
relatively sure we're already leaking sysfs_dirents by forgetting
group removals in some paths.
It's a broken interface and even for the existing users we should be
able to remove most of group group removal invocation afterwards,
which is a silly and error-prone requirement.
Thanks.
--
tejun
prev parent reply other threads:[~2013-09-27 13:49 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-18 21:15 Tejun Heo
2013-09-18 21:15 ` [PATCH 1/4] sysfs: remove sysfs_addrm_cxt->parent_sd Tejun Heo
2013-09-18 21:15 ` [PATCH 2/4] kobject: grab an extra reference on kobject->sd to allow duplicate deletes Tejun Heo
2013-09-18 21:15 ` [PATCH 3/4] sysfs: make __sysfs_remove_dir() recursive Tejun Heo
2013-09-18 21:15 ` [PATCH 4/4] sysfs: introduce [__]sysfs_remove() Tejun Heo
2013-09-19 10:48 ` [PATCHSET] sysfs: implement sysfs_remove() Eric W. Biederman
2013-09-19 12:38 ` Tejun Heo
2013-09-19 17:03 ` Eric W. Biederman
2013-09-26 23:44 ` Greg KH
2013-09-27 13:49 ` Tejun Heo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130927134924.GC3205@htj.dyndns.org \
--to=tj@kernel.org \
--cc=ebiederm@xmission.com \
--cc=gregkh@linuxfoundation.org \
--cc=kay@vrfy.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®