Re: [PATCHSET] sysfs: implement sysfs_remove()

From: Tejun Heo
Date: Fri Sep 27 2013 - 09:49:33 EST


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