Re: [PATCH] sysfs: move assignment to be under lock in sysfs_remove_dir()

[Tejun Heo <tj@kernel.org>]

Hey, Eric.

On Wed, Oct 30, 2013 at 02:41:16PM -0700, Eric W. Biederman wrote:
> Being nice here has resulted in buggy semantics exported to userspace,
> and buggy kernel code being written.  In a generalized version of sysfs
> we don't want to be nice to avoid the existing mess that sysfs sees.

Hmm... given that the generalized version always does recursive
deletion, I don't think the problem you described is too relevant.  It
really is something which can be either way.  Given that other
interfaces including sysfs_put() allow for NULL input, I think
flipping the behavior is likely more churn than worthwhile.

> Which given how sysfs is used is really insane. If we don't support
> something we need to warn about it, not 

Add the warning then?  Given that sysfs_remove() ends up putting the
base reference, it's something ultimately pointless tho.

> > Maybe we should rename the lock to sysfs_symlink_target_lock and add
> > fat comments on both sides?  Or we can make it a mutex and exclude the
> > entire removal and symlinking, which would probably easier to follow.
> 
> Agreed.  A mutex to excluse the entire removal and symlinking does sound
> easier to follow.  Why don't we call that mutex sysfs_mutex?

Well, we can merge any locking but that's likely to lead to some ugly
code.  sysfs core has always been somewhat isolated from kobject
integration and what you're suggesting would violate such isolation.
With the pending sysfs core separation, I don't think it's gonna work
at all.  kobj->sd association is something which is specific to kobj
<-> sysfs interaction.  If you wanna make behavior in that layer more
strictly synchronized, please go ahead but let's please not mix the
two layers.

Thanks.

-- 
tejun