All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Al Viro <viro-rfM+Q5joDG/XmaaqVzeoHQ@public.gmane.org>,
	Linux Containers
	<containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org>,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>,
	Benjamin Thery <benjamin.thery-6ktuUTfB/bM@public.gmane.org>
Subject: Re: [PATCH 06/11] sysfs: Implement sysfs tagged directory support.
Date: Thu, 03 Jul 2008 12:18:45 +0900	[thread overview]
Message-ID: <486C4515.1070007@gmail.com> (raw)
In-Reply-To: <m14p78s02q.fsf-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>

Hello, Eric.

Eric W. Biederman wrote:
>> If
>> changing the default function is too much of a hassle (and I'm sure it
>> would be), just add an extended version which takes @tag.  The current
>> implementation feels like it tried too hard to not add intermediate
>> interfaces and ended up shooting outside from the innermost layer.
> 
> It tried for something that was simple to use and that worked.
> 
> Also the way things work.  I have to use all of the intermediate layers
> and their calls to various functions.  So just passing a parameter through
> doesn't work to well.

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.

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

>>> On the other side I can't pass a tag through from the device layer to
>>> the kobject layer.  It isn't a concept the kobject layer supports.
>> I think it's best to make kobject layer support it.
> 
> Assuming Greg will accept it when he sees reasonable patches.

Greg says he would.  :-)

Thanks a lot for your patience.

-- 
tejun

WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <htejun@gmail.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Benjamin Thery <benjamin.thery@bull.net>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Daniel Lezcano <dlezcano@fr.ibm.com>,
	Serge Hallyn <serue@us.ibm.com>,
	linux-kernel@vger.kernel.org, Al Viro <viro@ftp.linux.org.uk>,
	Linux Containers <containers@lists.osdl.org>
Subject: Re: [PATCH 06/11] sysfs: Implement sysfs tagged directory support.
Date: Thu, 03 Jul 2008 12:18:45 +0900	[thread overview]
Message-ID: <486C4515.1070007@gmail.com> (raw)
In-Reply-To: <m14p78s02q.fsf@frodo.ebiederm.org>

Hello, Eric.

Eric W. Biederman wrote:
>> If
>> changing the default function is too much of a hassle (and I'm sure it
>> would be), just add an extended version which takes @tag.  The current
>> implementation feels like it tried too hard to not add intermediate
>> interfaces and ended up shooting outside from the innermost layer.
> 
> It tried for something that was simple to use and that worked.
> 
> Also the way things work.  I have to use all of the intermediate layers
> and their calls to various functions.  So just passing a parameter through
> doesn't work to well.

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.

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

>>> On the other side I can't pass a tag through from the device layer to
>>> the kobject layer.  It isn't a concept the kobject layer supports.
>> I think it's best to make kobject layer support it.
> 
> Assuming Greg will accept it when he sees reasonable patches.

Greg says he would.  :-)

Thanks a lot for your patience.

-- 
tejun

  parent reply	other threads:[~2008-07-03  3:18 UTC|newest]

Thread overview: 183+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-18 17:07 [PATCH 00/11] sysfs tagged directories V6 Benjamin Thery
2008-06-18 17:07 ` [PATCH 01/11] sysfs: Support for preventing unmounts Benjamin Thery
     [not found]   ` <20080618170730.256129077-4vkkeT0zb4ZEtYaxpPmRp1aPQRlvutdw@public.gmane.org>
2008-06-18 17:44     ` Dave Hansen
2008-06-18 17:44       ` Dave Hansen
2008-06-18 20:12       ` Eric W. Biederman
2008-06-19  8:54         ` Benjamin Thery
2008-06-19 16:32         ` Dave Hansen
2008-06-19 20:19           ` Benjamin Thery 
2008-06-18 17:07 ` [PATCH 02/11] sysfs: sysfs_get_dentry add a sb parameter Benjamin Thery
2008-06-18 17:07 ` [PATCH 03/11] sysfs: Implement __sysfs_get_dentry Benjamin Thery
2008-06-18 17:08 ` [PATCH 04/11] sysfs: Rename Support multiple superblocks Benjamin Thery
2008-06-18 17:08 ` [PATCH 05/11] sysfs: sysfs_chmod_file handle " Benjamin Thery
2008-06-22  4:46   ` Tejun Heo
2008-06-23 21:42     ` Daniel Lezcano
2008-06-24  4:45       ` Tejun Heo
2008-06-24 10:39         ` Daniel Lezcano
2008-06-24 13:37         ` Daniel Lezcano
2008-06-25 12:31           ` Tejun Heo
2008-06-18 17:08 ` [PATCH 06/11] sysfs: Implement sysfs tagged directory support Benjamin Thery
2008-06-23  2:05   ` Tejun Heo
2008-06-26 20:21     ` Eric W. Biederman
2008-06-29  3:51       ` Tejun Heo
2008-06-30 18:56         ` Eric W. Biederman
     [not found]           ` <m18wwmsqdv.fsf-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>
2008-06-30 21:44             ` Serge E. Hallyn
2008-06-30 21:44               ` Serge E. Hallyn
2008-07-01  7:50               ` Eric W. Biederman
2008-07-01  6:47           ` Tejun Heo
2008-07-01  9:20             ` Eric W. Biederman
2008-07-01 10:30               ` Tejun Heo
2008-07-01 12:30                 ` Eric W. Biederman
2008-07-02  3:24                   ` Tejun Heo
2008-07-02  3:53                     ` Eric W. Biederman
2008-07-02  4:37                       ` Tejun Heo
     [not found]                         ` <486B060C.7030607-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2008-07-02  7:18                           ` Andreas B Aaen
2008-07-02 16:49                         ` Eric W. Biederman
2008-07-03  0:15                           ` Greg KH
     [not found]                           ` <m14p78s02q.fsf-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>
2008-07-03  3:18                             ` Tejun Heo [this message]
2008-07-03  3:18                               ` Tejun Heo
     [not found]                               ` <486C4515.1070007-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2008-07-03  5:11                                 ` Eric W. Biederman
2008-07-03  5:11                                   ` Eric W. Biederman
     [not found]                                   ` <m1hcb7o8lv.fsf-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>
2008-07-03 10:56                                     ` Daniel Lezcano
2008-07-03 10:56                                       ` Daniel Lezcano
     [not found]                                       ` <486CB051.5000507-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>
2008-07-03 12:27                                         ` Eric W. Biederman
2008-07-03 12:27                                           ` Eric W. Biederman
     [not found]                                           ` <m14p77m9uy.fsf-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>
2008-07-03 12:37                                             ` Benjamin Thery
2008-07-03 12:37                                               ` Benjamin Thery
2008-07-03 19:57                                               ` Eric W. Biederman
2008-07-03 12:55                                             ` Daniel Lezcano
2008-07-03 12:55                                               ` Daniel Lezcano
2008-07-03 15:58                                           ` Tejun Heo
2008-07-03 18:29                                             ` Daniel Lezcano
2008-07-03 20:08                                             ` Eric W. Biederman
2008-07-04  0:48                                             ` [PATCH 00/15] sysfs support for namespaces Eric W. Biederman
2008-07-04  0:48                                               ` Eric W. Biederman
2008-07-04  1:05                                               ` [PATCH 01/15] kobject: Cleanup kobject_rename and !CONFIG_SYSFS Eric W. Biederman
2008-07-04  1:05                                                 ` Eric W. Biederman
2008-07-04  1:07                                                 ` [PATCH 02/15] sysfs: Support for preventing unmounts Eric W. Biederman
2008-07-04  1:07                                                   ` Eric W. Biederman
2008-07-04  1:08                                                   ` [PATCH 03/15] sysfs: sysfs_get_dentry add a sb parameter Eric W. Biederman
2008-07-04  1:08                                                     ` Eric W. Biederman
2008-07-04  1:09                                                     ` [PATCH 04/15] sysfs: Implement __sysfs_get_dentry Eric W. Biederman
2008-07-04  1:09                                                       ` Eric W. Biederman
2008-07-04  1:10                                                       ` [PATCH 05/15] sysfs: Rename Support multiple superblocks Eric W. Biederman
2008-07-04  1:10                                                         ` Eric W. Biederman
2008-07-04  1:11                                                         ` [PATCH 06/15] Introduce sysfs_sd_setattr and fix sysfs_chmod Eric W. Biederman
2008-07-04  1:11                                                           ` Eric W. Biederman
2008-07-04  1:13                                                           ` [PATCH 07/15] sysfs: sysfs_chmod_file handle multiple superblocks Eric W. Biederman
2008-07-04  1:13                                                             ` Eric W. Biederman
2008-07-04  1:14                                                             ` [PATCH 08/15] sysfs: Make sysfs_mount static once again Eric W. Biederman
2008-07-04  1:14                                                               ` Eric W. Biederman
2008-07-04  1:16                                                               ` [PATCH 09/15] sysfs: Implement sysfs tagged directory support Eric W. Biederman
2008-07-04  1:16                                                                 ` Eric W. Biederman
2008-07-04  1:17                                                                 ` [PATCH 10/15] sysfs: Merge sysfs_rename_dir and sysfs_move_dir Eric W. Biederman
2008-07-04  1:17                                                                   ` Eric W. Biederman
2008-07-04  1:18                                                                   ` [PATCH 11/15] sysfs: Implement sysfs_delete_link and sysfs_rename_link Eric W. Biederman
2008-07-04  1:18                                                                     ` Eric W. Biederman
2008-07-04  1:20                                                                     ` [PATCH 12/15] driver core: Implement tagged directory support for device classes Eric W. Biederman
2008-07-04  1:20                                                                       ` Eric W. Biederman
2008-07-04  1:21                                                                       ` [PATCH 13/15] Revert "netns: Fix device renaming for sysfs" Eric W. Biederman
2008-07-04  1:21                                                                         ` Eric W. Biederman
2008-07-04  1:22                                                                         ` [PATCH 14/15] netns: Enable tagging for net_class directories in sysfs Eric W. Biederman
2008-07-04  1:22                                                                           ` Eric W. Biederman
2008-07-04  1:23                                                                           ` [PATCH 15/15] sysfs: user namespaces: fix bug with clone(CLONE_NEWUSER) with fairsched Eric W. Biederman
2008-07-04  1:23                                                                             ` Eric W. Biederman
2008-07-04  7:50                                                                       ` [PATCH 12/15] driver core: Implement tagged directory support for device classes Tejun Heo
2008-07-04  7:50                                                                         ` Tejun Heo
2008-07-04 13:31                                                                         ` Eric W. Biederman
2008-07-04 13:57                                                                           ` Tejun Heo
2008-07-04 13:57                                                                             ` Tejun Heo
2008-07-04 16:12                                                                             ` Greg KH
2008-07-04 21:49                                                                               ` Eric W. Biederman
2008-07-14  1:54                                                                               ` Eric W. Biederman
2008-07-16  3:25                                                                                 ` Tejun Heo
2008-07-16  5:41                                                                                   ` Eric W. Biederman
2008-07-16  5:50                                                                                     ` Tejun Heo
2008-07-16  6:32                                                                                       ` Eric W. Biederman
2008-07-16  6:48                                                                                         ` Tejun Heo
2008-07-16  7:02                                                                                           ` Tejun Heo
2008-07-16 19:07                                                                                             ` Eric W. Biederman
2008-07-16 21:09                                                                                           ` Eric W. Biederman
2008-07-17 23:08                                                                                   ` Greg KH
2008-07-18 12:41                                                                                     ` Tejun Heo
2008-07-18 18:49                                                                                       ` Greg KH
2008-07-18 20:19                                                                                         ` Eric W. Biederman
2008-07-19  1:07                                                                                         ` Tejun Heo
2008-08-03  6:59                                                                                         ` Eric W. Biederman
2008-09-11 12:45                                                                                           ` Jiri Slaby
2008-09-11 13:05                                                                                             ` Benjamin Thery
2008-09-12  6:32                                                                                               ` Jiri Slaby
2008-07-04 22:00                                                                             ` Eric W. Biederman
2008-08-20  2:17                                                                 ` [PATCH 09/15] sysfs: Implement sysfs tagged directory support Greg KH
2008-08-20  6:58                                                                   ` Eric W. Biederman
2008-08-21  6:31                                                                   ` [PATCH 0/8] sysfs namespace support Eric W. Biederman
2008-08-21  6:33                                                                     ` [PATCH 1/8] sysfs: Implement sysfs tagged directory support Eric W. Biederman
2008-08-21  6:34                                                                       ` [PATCH 2/8] sysfs: Merge sysfs_rename_dir and sysfs_move_dir Eric W. Biederman
2008-08-21  6:35                                                                         ` [PATCH 3/8] sysfs: Implement sysfs_delete_link and sysfs_rename_link Eric W. Biederman
2008-08-21  6:36                                                                           ` [PATCH 5/8] sysfs: Remove sysfs_create_link_nowarn Eric W. Biederman
2008-08-21  6:38                                                                             ` [PATCH 6/8] Revert "netns: Fix device renaming for sysfs" Eric W. Biederman
2008-08-21  6:39                                                                               ` [PATCH 7/8] netns: Enable tagging for net_class directories in sysfs Eric W. Biederman
2008-08-21  6:40                                                                                 ` [PATCH 8/8] sysfs: user namespaces: fix bug with clone(CLONE_NEWUSER) with fairsched Eric W. Biederman
2008-08-21  6:47                                                                                 ` [PATCH 7/8] netns: Enable tagging for net_class directories in sysfs David Miller
2008-08-21  6:47                                                                               ` [PATCH 6/8] Revert "netns: Fix device renaming for sysfs" David Miller
2008-08-21  6:37                                                                           ` [PATCH 4/8] driver core: Implement tagged directory support for device classes Eric W. Biederman
2008-08-27 15:18                                                                       ` [PATCH 1/8] sysfs: Implement sysfs tagged directory support Benjamin Thery
2008-09-02 13:54                                                                         ` Mark Ryden
2008-09-02 14:03                                                                           ` Benjamin Thery
2008-09-02 17:01                                                                             ` Greg KH
2008-09-04  5:33                                                                               ` David Shwatrz
2008-09-04  6:44                                                                                 ` Benjamin Thery
2008-09-08 18:39                                                                                   ` Mark Ryden
2008-10-07 16:39                                                                                   ` Mark Ryden
2008-10-07 16:48                                                                                     ` Greg KH
2008-10-07 20:31                                                                                       ` Eric W. Biederman
2008-10-07 21:09                                                                                         ` Greg KH
2008-10-07 22:27                                                                                           ` Eric W. Biederman
2008-10-08 13:00                                                                                             ` Christoph Hellwig
2008-10-14  3:20                                                                                               ` Eric W. Biederman
2008-10-07 16:52                                                                                     ` Daniel Lezcano
2008-08-21  6:37                                                                     ` [PATCH 0/8] sysfs namespace support David Miller
2008-07-04  6:44                                                               ` [PATCH 08/15] sysfs: Make sysfs_mount static once again Tejun Heo
2008-07-04  6:44                                                             ` [PATCH 07/15] sysfs: sysfs_chmod_file handle multiple superblocks Tejun Heo
2008-08-20  2:16                                                             ` patch sysfs-sysfs_chmod_file-handle-multiple-superblocks.patch added to gregkh-2.6 tree gregkh
2008-08-20  2:16                                                               ` gregkh
2008-07-04  6:40                                                           ` [PATCH 06/15] Introduce sysfs_sd_setattr and fix sysfs_chmod Tejun Heo
2008-08-20  2:16                                                           ` patch sysfs-introduce-sysfs_sd_setattr-and-fix-sysfs_chmod.patch added to gregkh-2.6 tree gregkh
2008-08-20  2:16                                                             ` gregkh
2008-08-20  2:16                                                         ` patch sysfs-rename-support-multiple-superblocks.patch " gregkh
2008-08-20  2:16                                                           ` gregkh
2008-08-20  2:16                                                       ` patch sysfs-implement-__sysfs_get_dentry.patch " gregkh
2008-08-20  2:16                                                         ` gregkh
2008-08-20  2:16                                                     ` patch sysfs-sysfs_get_dentry-add-a-sb-parameter.patch " gregkh
2008-08-20  2:16                                                       ` gregkh
     [not found]                                                   ` <m1prpuihjb.fsf_-_-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>
2008-08-20  2:16                                                     ` patch sysfs-support-for-preventing-unmounts.patch " gregkh-l3A5Bk7waGM
2008-07-04  6:33                                                 ` [PATCH 01/15] kobject: Cleanup kobject_rename and !CONFIG_SYSFS Tejun Heo
2008-08-20  1:48                                                 ` patch kobject-cleanup-kobject_rename-and-config_sysfs.patch added to gregkh-2.6 tree gregkh
2008-08-20  1:48                                                   ` gregkh
2008-07-04  1:27                                               ` [PATCH 00/15] sysfs support for namespaces Eric W. Biederman
2008-07-04  1:27                                                 ` Eric W. Biederman
2008-07-06  4:42                                               ` Eric W. Biederman
2008-07-06  4:42                                                 ` Eric W. Biederman
2008-07-07 11:41                                                 ` Cornelia Huck
2008-07-07 11:41                                                   ` Cornelia Huck
2008-07-07 12:22                                                   ` Eric W. Biederman
2008-07-07 12:22                                                     ` Eric W. Biederman
2008-06-18 17:08 ` [PATCH 07/11] sysfs: Implement sysfs_delete_link and sysfs_rename_link Benjamin Thery
2008-06-23  2:13   ` Tejun Heo
2008-06-26 20:24     ` Eric W. Biederman
2008-06-29  3:35       ` Tejun Heo
2008-06-30  3:02         ` Eric W. Biederman
2008-06-18 17:08 ` [PATCH 08/11] driver core: Implement tagged directory support for device classes Benjamin Thery
2008-06-18 17:08 ` [PATCH 09/11] sysfs: add sysfs_ns_exit routine Benjamin Thery
2008-06-18 20:19   ` Eric W. Biederman
2008-06-23  2:16   ` Tejun Heo
2008-06-18 17:09 ` [PATCH 10/11] netns: Enable tagging for net_class directories in sysfs Benjamin Thery
2008-06-23  2:18   ` Tejun Heo
2008-06-18 17:09 ` [PATCH 11/11] sysfs: user namespaces: fix bug with clone(CLONE_NEWUSER) with fairsched Benjamin Thery
2008-06-23  2:18   ` Tejun Heo
2008-06-25 18:44     ` Serge E. Hallyn
2008-06-25 21:11       ` Eric W. Biederman
     [not found]         ` <m1wskd43uc.fsf-B27657KtZYmhTnVgQlOflh2eb7JE58TQ@public.gmane.org>
2008-06-26 13:07           ` Serge E. Hallyn
2008-06-26 13:07             ` Serge E. Hallyn
  -- strict thread matches above, loose matches on Subject: below --
2008-06-06 15:46 [PATCH 00/11] sysfs tagged directories V5 Benjamin Thery
2008-06-06 15:47 ` [PATCH 06/11] sysfs: Implement sysfs tagged directory support Benjamin Thery
2008-05-06 17:30 [RESEND][PATCH 00/11] sysfs tagged directories Benjamin Thery
2008-05-06 17:31 ` [PATCH 06/11] sysfs: Implement sysfs tagged directory support Benjamin Thery

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=486C4515.1070007@gmail.com \
    --to=htejun-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=benjamin.thery-6ktuUTfB/bM@public.gmane.org \
    --cc=containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org \
    --cc=dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=gregkh-l3A5Bk7waGM@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=viro-rfM+Q5joDG/XmaaqVzeoHQ@public.gmane.org \
    /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.