Re: [PATCHSET 3/4] sysfs: divorce sysfs from kobject and driver model

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Tejun Heo
Date: Friday, October 5, 2007 - 1:00 am

Hello, Greg.

I think this definitely needs more discussion, so here we go...

Greg KH wrote:
[--snip--]
[--snip--]

Yes, exactly - "internally" to the driver model (or drivers which ride
along).  To sysfs, it has no function other than being an opaque token.

[--snip--]

Yes, which only needs to be used _inside_ the driver model
implementation proper.


Things can be much easier than now.  Also, the above paragraph is
inconsistent with the rest of your argument or am I misunderstanding
what you mean by the above paragraph?


This is and should be the function of the driver model not kobjects.
Removing kobject from the interface doesn't change anything about this.


To me, this just feels wrong and does more harm than it helps.  I really
think we shouldn't have multi-role flash light at the core of our driver
model (inside driver model proper, no problem, but not as exported
interface).


For #1 and #3, I agree if you limit the scope to driver model proper but
I am not arguing kobject and all its friends should be abolished.  I'm
arguing that there is no reason to export it as API because it doesn't
add any value exported.

For #4, I don't know.  This can be a matter of taste but I don't think
#4 alone stands as justification for kobject as external API.

For #2, I might be misundertanding.  Please elaborate if I am.  For
uevent issue, I'll talk about more later.

So, I honestly don't think the above four arguments successfully counter
the original arguments.  If I'm missing something, feel free to hammer
me into enlightenment.  :-)


There is no code reduction or functionality improvement yet because all
of them are still using the compatibility interface.  Properly
converted, sysfs handling code all over the kernel can be _much_
simplified and more robust.  I bet there are numerous bugs in sysfs
creation failure handling path all over mid/low level drivers.  New
interface makes those bugs much less likely.


The thing is that functionality-wise, kobject and its friends don't
serve anything anymore outside of driver model implementation proper
(I'll talk about uevent later) and thus there is no reason to use it
outside of driver model implementation anymore in the long term.

If something is needed but bypassed, it's circumvented but that isn't
the case here.  kobject and its friends no longer have any essential
functionality in the exported API.  It's just a dead weight. (Any entity
in the driver model can and should use what the driver model exports, so
that part is irrelevant here.)


I can agree to that.  Unfortunately, it also sometimes distorts driver
implementation because representing the proper picture is so difficult.
 libata attributes are under constant pressure to escape to SCSI and
block nodes (nothing bad has actually happened yet tho), new features
are being delayed and/or pushed to use different userland interface
(module parameter being the most common).  I know libata is a corner
case at the moment but a bit of flexibility would have been very helpful
for both developers and users.


Yes, fully agreed.  What I'm trying to argue is that obfuscation isn't
the optimal way to achieve that.  We can do it in saner and less painful
way.

[--snip--]

Adding policies to prevent such usages to easy interface is the right
thing to do.  Currently, we don't even have defined policies for sysfs
outside of driver model.  The only thing is that it's difficult to
understand and painful to use.

I just don't really get how it's okay to keep kobject based interface
just to make things more difficult and solely depend on that artificial
difficulty for keeping the tree tidy.

We can enforce stronger rules with easier interface.  Just lemme know
the rules.  I'll enforce those rules in the sysfs core such that
changing those rules will have to go through driver model review chain.
 Wouldn't that be much better?


Yes and sysfs restructuring when it's finished won't change that at all.
 Things will be better toward the same goal.  Remember that I said the
next step was moving uevent over to sysfs?  Uevent belongs to sysfs
because it's by design bound to userland visible representation of
kernel objects.  The current placement is awkward - kobject carries
uevent related fields whether it's needed or not, uevent suppression is
in struct device not in kobject and sysfs creation / uevent
synchronization is done in awkward way.


I think what's missing here is why we need to enforce kobject interface.
 It certainly isn't for kobject itself's sake, right?  Originally, it
served a valid purpose for interaction with sysfs.  Also, by the virtue
of being difficult to use, it limited the usage of sysfs.

My arguments here are...

1. kobject no longer has such valid purpose as far as sysfs is
concerned, which was its biggest out-of-driver-model functionality.
And, in the long term, I don't see any reason why kobject needs to be
visible outside of driver model.

2. I'm all for keeping the tree tidy but I think it's better and more
cleanly done by well defined policies clearly stated in the
documentation and enforced by code such that changing sysfs hierarchy
always goes through driver model review chain.

3. In this series, all that happened was implementing new interface and
features and reimplement original interface in terms of them.  As such,
there is no code clean up out of sysfs.  In fact, sysfs gained
considerable amount of code.  Considering wide spread use of sysfs, I'm
pretty sure the net code amount and complexity will drop considerably
with future API user conversions.

Hopefully, I stated things clearer this time.  If you disagree, please
try to convince me.  I'm listening and I think we really need to
establish consensus on this subject.

Thanks a lot.

-- 
tejun
-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 05/22] sysfs: implement sysfs_find_child(), Tejun Heo, (Thu Sep 20, 1:05 am)
[PATCH 06/22] sysfs: restructure addrm helpers, Tejun Heo, (Thu Sep 20, 1:05 am)
[PATCH 01/22] sysfs: make sysfs_root a pointer, Tejun Heo, (Thu Sep 20, 1:05 am)
[PATCH 04/22] sysfs: make SYSFS_COPY_NAME a flag, Tejun Heo, (Thu Sep 20, 1:05 am)
[PATCH 14/22] sysfs: s/symlink/link/g, Tejun Heo, (Thu Sep 20, 1:05 am)
[PATCH 21/22] sysfs: kill sysfs_hash_and_remove(), Tejun Heo, (Thu Sep 20, 1:05 am)
Re: [PATCHSET 3/4] sysfs: divorce sysfs from kobject and d ..., Eric W. Biederman, (Thu Sep 27, 12:25 pm)
Re: [PATCHSET 3/4] sysfs: divorce sysfs from kobject and d ..., Tejun Heo, (Fri Oct 5, 1:00 am)
Re: [PATCHSET 3/4] sysfs: divorce sysfs from kobject and d ..., Eric W. Biederman, (Wed Oct 10, 6:16 am)
Re: [PATCHSET 3/4] sysfs: divorce sysfs from kobject and d ..., Eric W. Biederman, (Wed Oct 10, 2:16 pm)
Re: [PATCHSET 3/4] sysfs: divorce sysfs from kobject and d ..., Eric W. Biederman, (Tue Oct 16, 4:54 pm)