All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Tejun Heo <htejun@gmail.com>,
	ebiederm@xmission.com, stern@rowland.harvard.edu,
	kay.sievers@vrfy.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHSET 3/4] sysfs: divorce sysfs from kobject and driver model
Date: Tue, 9 Oct 2007 15:26:38 -0700	[thread overview]
Message-ID: <20071009222638.GD21228@kroah.com> (raw)
In-Reply-To: <20071009112901.6d4fa19d@gondolin>

On Tue, Oct 09, 2007 at 11:29:01AM +0200, Cornelia Huck wrote:
> On Fri, 05 Oct 2007 17:00:48 +0900,
> Tejun Heo <htejun@gmail.com> wrote:
> 
> > I think this definitely needs more discussion, so here we go...
> 
> I agree, so I'll give my 0.02 ? here...
> > 
> > Greg KH wrote:
> > >> 1. What is a kobject?
> > [--snip--]
> > >> The functionality served by kobject can be broken down into the
> > >> following two.
> > >>
> > >> F-a. To serve as an entity both subsystems can share lifespan
> > >> management.  ie. both subsystems reference count on kobject.
> > >>
> > >> F-b. To serve as an entity both subsystems can base their internal
> > >> representation on.  (base object in OO term).
> > > 
> > > Yes, those are two functions, I can agree with.
> 
> I think that's the heart of the question: We first need to agree what
> use the different components should have.
> 
> (a) The driver model
> 
> The driver model serves as a unified layer for all devices managed by
> the kernel, organized in trees, and the drivers handling them. This
> includes busses, matching of devices and drivers, attributes and so on.
> Userspace expects to see these devices, drivers, busses and attributes
> by looking under /sys/devices/. /sys/class/ and /sys/bus/ provide
> additional views on this data.
> 
> (b) kobjects, ktypes, ksets
> 
> kobjects provide a mechanism to arrange kernel objects into a tree-like
> structure. ktypes and ksets are mechanisms to further order these
> objects. Changes in the kobject hierarchy are communicated to userspace
> via uevents.
> 
> The driver core is implemented using this infrastructure.
> 
> (c) krefs
> 
> krefs provide a generic reference counting mechanism.
> 
> The kobject infrastructure uses krefs for its reference counting needs.
> 
> (d) sysfs
> 
> sysfs is a virtual filesystem. It exports information on kernel objects
> to user space. (IMO, that's the key: sysfs is userspace representation.)

Ok, I agree with all of the above :)

> That said, it is logical that kobjects are made visible to userspace
> via sysfs. If someone is trying to make things show up in sysfs and has
> to jump through hoops to cook up kobjects, they're probably using the
> wrong infrastructure.

I agree.

> There are two big problems with the tight coupling of sysfs and kobjects:
> 
> - lifetime rules; but this fortunately hugely improved with the
> previous patches :)

Yes, I think that's pretty much fixed now.

> - relaying implementation details to userspace so that they cannot be
> easily changed. We would need to allow kobjects not showing up in sysfs
> and making symlinks a sysfs facility not relying on kobjects to help
> there.

Huh?  Why would you want a kobject to not show up in sysfs?

And yes, I agree we could use some more "automatic" help in regards to
symlinks in sysfs when we change kobjects around.  That would make
things simpler for the kobject core.

> > [--snip--]
> > >> 3. Where does that leave kobject?
> > >>
> > >> If you combine #1 and #2, both functionalities become questionable.
> > >>
> > >> F-a. sysfs no longer plays role in lifespan management of driver model
> > >> object.  This functionality is exactly what's killed by #2.
> > > 
> > > Yes, but a kobject is still needed internally for the lifespan
> > > management.
> > 
> > Yes, exactly - "internally" to the driver model (or drivers which ride
> > along).  To sysfs, it has no function other than being an opaque token.
> 
> But krefs are used for kobject reference counting, or am I
> misunderstanding here?
> 
> > 
> > [--snip--]
> > >> IMHO, both L-a and L-b contribute only to obfuscation of the driver
> > >> model and sysfs.
> > > 
> > > No.  I think you are missing a number of things that kobjects provide
> > > and allow:
> > > 	- a structurual heirachy of devices.  Combine kobjects with
> > > 	  ksets and ktypes, and you have a very powerful system to
> > > 	  categorize objects and their representation to each other.
> > 
> > Yes, which only needs to be used _inside_ the driver model
> > implementation proper.
> 
> There are use cases outside of the driver model prober where you may
> want to use kobject for hierarchy.

agreed.

> > > 	- a consistant and easy interface to userspace through
> > > 	  uevents/hotplug of the creation and removal of these objects.
> > > 	  This keeps the different parts of the kernel that need this
> > > 	  interface from having to create it every time on their own.
> > 
> > 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?
> 
> I see uevents as a notifier for changes in the kobject hierarchy, so
> they belong to that layer. However, the layering between kobjects and
> sysfs (path names etc.) could probably be made cleaner.

also agreed.

> > > 	- a way to easily create and export attributes in sysfs
> > > 	  automatically.
> > 
> > This is and should be the function of the driver model not kobjects.
> 
> I agree, attributes should belong to the driver model.
> 
> > Removing kobject from the interface doesn't change anything about this.
> 
> Hm. Currently you have a file<->attribute correlation. This would
> change if you allow non-attribute files.

I don't want to have non-attribute files, that's my main point here.

> > > 	- a way to provide working reference counting for a variety of
> > > 	  different objects.
> > 
> > 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).
> 
> And I still think that this is the purpose of krefs :)

Ok, yes, at the very base level, it is, you are correct :)

> > > All of those are still needed for the kernel.
> > 
> > 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.
> 
> I see the value for those code paths that want to provide a hierarchy
> of kernel objects outside the driver model proper.

Yes.

> > >> The rest isn't great in number and, much more importantly, many of those
> > >> suffer from the current interface which is painful to use independently.
> > >>  For example, kernel/module.c does all the kobject dances including
> > >> defining a subsystem just to ignore everything else and use it as an
> > >> opaque token to sysfs (kset_find_obj doesn't count, a generic map or
> > >> sysfs with sysfs_dirent interface can do that just as well).
> > > 
> > > I will not deny that the current use of kobjects/ksets/ktypes (subsystem
> > > is now gone) is difficult and extreemly painful.  I am currently working
> > > to fix this issue.  But don't think that the reason this is hard to use
> > > means that it should be abolished alltogether.
> > > 
> > > Rather, it means that this interface to using kobjects needs to be fixed
> > > and made easier, not circumvented.
> 
> Yes, an easier-to-use interface to the kobject stuff would be helpful
> for everyone :)
> 
> > 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.
> 
> I disagree. A hierarchy of kernel objects has uses beyond the driver
> model.

i agree.

thanks,

greg k-h

  reply	other threads:[~2007-10-09 22:58 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-20  8:05 [PATCHSET 3/4] sysfs: divorce sysfs from kobject and driver model Tejun Heo
2007-09-20  8:05 ` [PATCH 01/22] sysfs: make sysfs_root a pointer Tejun Heo
2007-09-20  8:05 ` [PATCH 06/22] sysfs: restructure addrm helpers Tejun Heo
2007-09-20  8:05 ` [PATCH 05/22] sysfs: implement sysfs_find_child() Tejun Heo
2007-09-20  8:05 ` [PATCH 02/22] sysfs: separate out sysfs-kobject.h and fs/sysfs/kobject.c Tejun Heo
2007-09-20  8:05 ` [PATCH 04/22] sysfs: make SYSFS_COPY_NAME a flag Tejun Heo
2007-09-20  8:05 ` [PATCH 03/22] sysfs: make sysfs_new_dirent() normalize @mode and determine file type Tejun Heo
2007-09-20  8:05 ` [PATCH 10/22] sysfs: drop kobj and attr from file related symbols Tejun Heo
2007-09-20  8:05 ` [PATCH 11/22] sysfs: implement sysfs_dirent based file interface Tejun Heo
2007-09-20  8:05 ` [PATCH 14/22] sysfs: s/symlink/link/g Tejun Heo
2007-09-20  8:05 ` [PATCH 08/22] sysfs: implement sysfs_dirent based directory interface Tejun Heo
2007-09-20  8:05 ` [PATCH 12/22] sysfs: drop kobj and attr from bin related symbols Tejun Heo
2007-09-20  8:05 ` [PATCH 13/22] sysfs: implement sysfs_dirent based bin interface Tejun Heo
2007-09-20  8:05 ` [PATCH 09/22] sysfs: rename internal function sysfs_add_file() Tejun Heo
2007-09-20  8:05 ` [PATCH 07/22] sysfs: implement sysfs_dirent based remove interface sysfs_remove() Tejun Heo
2007-09-20  8:05 ` [PATCH 18/22] kobject: implement __kobject_set_name() Tejun Heo
2007-09-20  8:05 ` [PATCH 16/22] sysfs: convert group implementation to use sd-based interface Tejun Heo
2007-09-20  8:05 ` [PATCH 15/22] sysfs: implement sysfs_dirent based link interface Tejun Heo
2007-09-20  8:05 ` [PATCH 17/22] sysfs: s/sysfs_rename_mutex/sysfs_op_mutex/ and protect all tree modifying ops Tejun Heo
2007-09-20  8:05 ` [PATCH 22/22] sysfs: move sysfs_assoc_lock into fs/sysfs/kobject.c and make it static Tejun Heo
2007-09-20  8:05 ` [PATCH 21/22] sysfs: kill sysfs_hash_and_remove() Tejun Heo
2007-09-20  8:05 ` [PATCH 19/22] sysfs: implement sysfs_dirent based rename - sysfs_rename() Tejun Heo
2007-09-20  8:05 ` [PATCH 20/22] sysfs: kill now unused __sysfs_add_file() Tejun Heo
2007-09-25 22:17 ` [PATCHSET 3/4] sysfs: divorce sysfs from kobject and driver model Greg KH
2007-09-27 11:35   ` Tejun Heo
2007-09-27 19:25     ` Eric W. Biederman
2007-09-29 22:06       ` Tejun Heo
2007-10-05  6:23       ` Greg KH
2007-10-05 12:12         ` Eric W. Biederman
2007-10-05 13:03           ` [Devel] " Kirill Korotaev
2007-10-05 13:24             ` Eric W. Biederman
2007-10-09 22:51           ` Greg KH
     [not found]             ` <20071009225139.GF21228-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2007-10-10 13:16               ` Eric W. Biederman
2007-10-10 13:16                 ` Eric W. Biederman
2007-10-10 20:44                 ` Greg KH
2007-10-10 21:16                   ` Eric W. Biederman
2007-10-16 22:18                 ` sukadev
2007-10-16 23:54                   ` Eric W. Biederman
2007-10-05 12:44         ` Eric W. Biederman
2007-10-09 22:53           ` Greg KH
2007-10-05  6:18     ` Greg KH
2007-10-05  8:00       ` Tejun Heo
2007-10-09  9:29         ` Cornelia Huck
2007-10-09 22:26           ` Greg KH [this message]
2007-10-09 23:20             ` Roland Dreier
2007-10-09 23:28               ` Greg KH
2007-10-10  9:11                 ` Cornelia Huck
2007-10-10  9:05             ` Cornelia Huck
2007-10-09 22:48         ` Greg KH
2007-10-10 15:38           ` Alan Stern
2007-10-10 16:16             ` Cornelia Huck
2007-10-10 17:24           ` Martin Bligh
2007-10-10 17:30             ` Greg KH
2007-10-10 18:26               ` Martin Bligh
2007-10-10 18:44                 ` Greg KH

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=20071009222638.GD21228@kroah.com \
    --to=greg@kroah.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=ebiederm@xmission.com \
    --cc=htejun@gmail.com \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.