netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Greg KH <gregkh@suse.de>,
	bcrl@lhnet.ca, benjamin.thery@bull.net, cornelia.huck@de.ibm.com,
	eric.dumazet@gmail.com, kay.sievers@vrfy.org,
	netdev@vger.kernel.org, serue@us.ibm.com
Subject: Re: patch sysfs-implement-sysfs-tagged-directory-support.patch added to gregkh-2.6 tree
Date: Fri, 30 Apr 2010 07:37:20 +0200	[thread overview]
Message-ID: <4BDA6C90.9010303@kernel.org> (raw)
In-Reply-To: <m1aaslbjh4.fsf@fess.ebiederm.org>

Hello,

On 04/30/2010 07:24 AM, Eric W. Biederman wrote:
>>> I wish at least more comments are added before it goes mainline.  I
>>> don't really understand the current form.
>>
>> Ok, that's fine with me, I'll pull it back out.
> 
> ?????
> 
> Tejun you have offered nothing constructive to the review, except looking
> and saying you don't understand what is going on.

Eric, no need to get too touchy and you're right in part in saying all
I'm saying is basically "I don't understand it" which is the same
reason why I'm not nacking it and explicitly stated that I would be
okay with the series going in if Greg/Kay would be okay with it.
Again, about the same thing with the above comment, I was *wishing*
for more comments *before it goes mainline*.

> Tejun I think for the code to make any sense to you I would need to rip
> out out and/or rewrite the kobject layer, and possible the device
> model code as well.

And yes, in the long run, please do that.

> Tejun I'm sorry you can't understand the code, and I'm sorry the code
> may be over-general.  In part that is because making the code
> over-general is what you asked for when reviewing it the first time.

Please give me some credit.  I mean that the code is difficult to
follow and justify when I say I don't understand it.  Yeah, I tried to
understand it and I think I understand how it *works* in its current
form but I just don't think the design is justified or logical.  You
say it's infeasible to do it in straightforward manner in reasonable
amount of time and that's why I neither acked or nacked the series and
deferred the decision to the subsystem maintainer.

But, at the very least, please add some comments.  Try to explain what
each callbacks are supposed to do and why they're there.  Not everyone
lives in your head.

Thanks.

-- 
tejun

  reply	other threads:[~2010-04-30 17:04 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-30 18:30 [PATCH 0/6] tagged sysfs support Eric W. Biederman
2010-03-30 18:31 ` [PATCH 1/6] sysfs: Basic support for multiple super blocks Eric W. Biederman
2010-03-30 19:23   ` Eric Dumazet
2010-03-30 23:50     ` [PATCH 7/6] sysfs: Remove double free sysfs_get_sb Eric W. Biederman
2010-03-31  5:01   ` [PATCH 1/6] sysfs: Basic support for multiple super blocks Serge E. Hallyn
2010-03-31  5:01     ` Serge E. Hallyn
2010-03-31  5:41   ` Tejun Heo
2010-03-31  5:51     ` Eric W. Biederman
2010-03-31 13:47       ` Serge E. Hallyn
2010-03-31 14:02         ` Eric W. Biederman
2010-04-05  7:45       ` Tejun Heo
2010-04-29 20:29   ` patch sysfs-basic-support-for-multiple-super-blocks.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 2/6] kobj: Add basic infrastructure for dealing with namespaces Eric W. Biederman
2010-04-29 20:29   ` patch kobj-add-basic-infrastructure-for-dealing-with-namespaces.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 3/6] sysfs: Implement sysfs tagged directory support Eric W. Biederman
2010-03-31  2:43   ` Serge E. Hallyn
2010-03-31  3:38     ` Eric W. Biederman
2010-03-31  4:02       ` Serge E. Hallyn
2010-03-31  4:23         ` Eric W. Biederman
2010-03-31  4:53           ` Serge E. Hallyn
2010-03-31  6:49   ` Tejun Heo
2010-03-31  7:43     ` Eric W. Biederman
2010-03-31  8:17       ` Tejun Heo
2010-03-31  8:22         ` Tejun Heo
2010-03-31  9:39           ` Eric W. Biederman
2010-04-05  8:17             ` Tejun Heo
2010-04-29 20:29   ` patch sysfs-implement-sysfs-tagged-directory-support.patch added to gregkh-2.6 tree gregkh
2010-04-30  4:18     ` Tejun Heo
2010-04-30  4:45       ` Greg KH
2010-04-30  5:24         ` Eric W. Biederman
2010-04-30  5:37           ` Tejun Heo [this message]
2010-04-30  6:12             ` Tejun Heo
2010-04-30 14:29             ` Serge E. Hallyn
2010-04-30 15:22               ` Tejun Heo
2010-04-30 15:43                 ` Serge E. Hallyn
2010-04-30 15:58                   ` Greg KH
2010-03-30 18:31 ` [PATCH 4/6] sysfs: Add support for tagged directories with untagged members Eric W. Biederman
2010-04-29 20:29   ` patch sysfs-add-support-for-tagged-directories-with-untagged-members.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 5/6] sysfs: Implement sysfs_delete_link Eric W. Biederman
2010-04-29 20:29   ` patch sysfs-implement-sysfs_delete_link.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:31 ` [PATCH 6/6] driver core: Implement ns directory support for device classes Eric W. Biederman
2010-04-29 20:29   ` patch driver-core-implement-ns-directory-support-for-device-classes.patch added to gregkh-2.6 tree gregkh
2010-03-30 18:53 ` [PATCH 0/6] tagged sysfs support Kay Sievers
2010-03-30 23:04   ` Eric W. Biederman
2010-03-31  5:51     ` Kay Sievers
2010-03-31  6:25       ` Tejun Heo
2010-03-31  6:52       ` Eric W. Biederman
2010-04-03  0:58       ` Ben Hutchings
2010-04-03  8:35         ` Kay Sievers
2010-04-03 16:05           ` Ben Hutchings
2010-04-03 16:35             ` Kay Sievers
2010-03-31 17:21 ` Serge E. Hallyn
2010-03-31 18:09   ` Eric W. Biederman
2010-05-05  0:35 ` [PATCH 0/6] netns support in the kobject layer Eric W. Biederman
2010-05-06 20:04   ` Greg KH
2010-05-16  6:26     ` David Miller
2010-05-17 18:11       ` Greg KH
2010-05-17 20:58         ` Eric W. Biederman
2010-05-17 21:03           ` Greg KH
2010-05-17 22:37             ` Eric W. Biederman
2010-05-17 22:54               ` Greg KH
2010-05-17 23:48             ` David Miller
2010-05-18  4:08               ` Greg KH
2010-05-18  4:21                 ` David Miller
2010-05-05  0:36 ` [PATCH 1/6] kobject: Send hotplug events in all network namespaces Eric W. Biederman
2010-05-20 18:10   ` patch kobject-send-hotplug-events-in-all-network-namespaces.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 2/6] netns: Teach network device kobjects which namespace they are in Eric W. Biederman
2010-05-05 15:17   ` Serge E. Hallyn
2010-05-05 19:56     ` Eric W. Biederman
2010-05-05 22:01       ` Serge E. Hallyn
2010-05-17  4:59         ` [PATCH 7/6] net/sysfs: Fix the bitrot in network device kobject namespace support Eric W. Biederman
2010-05-17  5:07           ` David Miller
2010-05-20 18:10   ` patch netns-teach-network-device-kobjects-which-namespace-they-are-in.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 3/6] netlink: Implment netlink_broadcast_filtered Eric W. Biederman
2010-05-20 18:10   ` patch netlink-implment-netlink_broadcast_filtered.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 4/6] kobj: Send hotplug events in the proper namespace Eric W. Biederman
2010-05-20 18:10   ` patch kobj-send-hotplug-events-in-the-proper-namespace.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 5/6] hotplug: netns aware uevent_helper Eric W. Biederman
2010-05-20 18:10   ` patch hotplug-netns-aware-uevent_helper.patch added to gregkh-2.6 tree gregkh
2010-05-05  0:36 ` [PATCH 6/6] net: Expose all network devices in a namespaces in sysfs Eric W. Biederman
2010-05-20 18:10   ` patch net-expose-all-network-devices-in-a-namespaces-in-sysfs.patch added to gregkh-2.6 tree gregkh
2010-05-20 17:47 ` [PATCH 0/6] tagged sysfs support 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=4BDA6C90.9010303@kernel.org \
    --to=tj@kernel.org \
    --cc=bcrl@lhnet.ca \
    --cc=benjamin.thery@bull.net \
    --cc=cornelia.huck@de.ibm.com \
    --cc=ebiederm@xmission.com \
    --cc=eric.dumazet@gmail.com \
    --cc=gregkh@suse.de \
    --cc=kay.sievers@vrfy.org \
    --cc=netdev@vger.kernel.org \
    --cc=serue@us.ibm.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).