Date:	Thu, 03 Jul 2008 12:56:17 +0200
From:	Daniel Lezcano <dlezcano@...ibm.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
CC:	Tejun Heo <htejun@...il.com>, Greg Kroah-Hartman <gregkh@...e.de>,
	linux-kernel@...r.kernel.org, Al Viro <viro@....linux.org.uk>,
	Linux Containers <containers@...ts.osdl.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Benjamin Thery <benjamin.thery@...l.net>
Subject: Re: [PATCH 06/11] sysfs: Implement sysfs tagged directory support.

Eric W. Biederman wrote:
> Tejun Heo <htejun@...il.com> writes:
> 
>> There is rather large possibility that I'm just being dumb here
>> especially because I haven't reviewed the users of this facility, so all
>> the comments I'm making are from the POV of interfaces of sysfs and the
>> related layers.  I think I've made my concerns clear by now.  If you
>> still think the callbacks are the best way to go, please try to
>> enlighten me.  I really don't wanna be stopping something which is
>> better from ignorance.  Just give me some concrete examples or point me
>> to codes which show how and why the current interface is the best for
>> the users and switching isn't a good idea.
> 
> Currently I think a callback on to get the tag from a kobject is the
> best way to go.  That way we don't need to add a field to struct
> kobject (and don't need the associated redundancy), and we can lookup
> up the tag when we need it.

The kobject events are sent through a netlink message which is not 
currently per network namespace. Shouldn't be useful to have a way to 
retrieve from the kobject the network namespace or the uevent socket 
associated with it ? IMHO having idr in the kobject + netns pointer 
associated may help to handle the sysfs isolation and makes the uevent 
per namespace trivial, no ?

> I have been playing with the code and just about have it ready
> to go.  I just need to refactor all of my changes into clean
> patches at this point, plus a bit of review and test.  Ben & Daniel
> have given me a version of the previous patchset rebased unto the
> latest -mm so that should help for the unchanged parts.
> 
> Introducing the sysfs_tag_type thing and pushing the functions to
> the edges helps.  It especially cleans up the ugly mount/umount
> situation allowing us to handle that with generic code.
> 
> Moving the kobject_tag into struct ktype works and looks roughly
> as clean as what happens with attributes.  So I that seems reasonable,
> and doesn't result in a significant change in the users.
> 
> The result of which means that I only have the helper function sysfs_creation_tag
> left in sysfs/dir.c  Left in there are some of the nasties in dealing with symlinks.
> 
> At this point I believe I have achieved a nice degree of simplifying the sysfs
> code in the current patches without really changing the users or
> making it more complex for them.
> 
> I have not implemented ida tags, and I don't plan to.  That is just
> unnecessary work right now.  The users are simple and the meat of the
> logic would not change so it should be simple to add.
> 
>>> It looks to me like the clean solution is move kobject_tag into
>>> kobj_type, and have it call some higher level function.
>>>
>>> We also need to remove the maintenance disaster that is
>>> kobject_set_name from sysfs_rename_dir.  And push it into
>>> kobject_rename instead.  The error handling is harder in
>>> that case but otherwise we should be in good shape.
>> Heh... I personally think kobject layer as a whole should just be hidden
>> under the cabinet of device driver model but I'm having difficult time
>> convincing other people of it.  Anyways, fully agree the interaction
>> between kobject and sysfs is ugly at a lot of places.
> 
> I would be happy if we could remove all nonsense kobject that are there just
> for structural purposes but have no purpose otherwise.  Things like kobjects
> for symlinks.  The kobject layer doesn't seem to have a clear identity
> and purpose that I can see right now.
> 
>> Thanks a lot for your patience.
> 
> Welcome.  The code reached a point a while ago where it didn't make sense
> to change it without review feedback.
> 
> Eric
> 
> _______________________________________________
> Containers mailing list
> Containers@...ts.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
> 


-- 






















































Sauf indication contraire ci-dessus:
Compagnie IBM France
Siège Social : Tour Descartes, 2, avenue Gambetta, La Défense 5, 92400
Courbevoie
RCS Nanterre 552 118 465
Forme Sociale : S.A.S.
Capital Social : 542.737.118 ?
SIREN/SIRET : 552 118 465 02430
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists