linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>,
	Kay Sievers <kay.sievers@vrfy.org>,
	linux-kernel@vger.kernel.org,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	linux-fsdevel@vger.kernel.org,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Benjamin LaHaise <bcrl@lhnet.ca>, Serge Hallyn <serue@us.ibm.com>,
	netdev@vger.kernel.org, Benjamin Thery <benjamin.thery@bull.net>
Subject: Re: [PATCH 3/6] sysfs: Implement sysfs tagged directory support.
Date: Mon, 05 Apr 2010 17:17:32 +0900	[thread overview]
Message-ID: <4BB99C9C.4070308@gmail.com> (raw)
In-Reply-To: <m1oci4zv7h.fsf@fess.ebiederm.org>

Hello, Eric.

On 03/31/2010 06:39 PM, Eric W. Biederman wrote:
> Let me try a happy median between overwhelming and too little
> information by giving you some experts, and a bit of overview.
> 
> (Ugh after have writing this I certainly will agree that we
>  have some many layers in the device model that they become
>  obfuscating abstractions).

Yeah, exactly, and this patchset is pushing it further with no
documentation and indirections to high heavens.  As someone who
doesn't have much experience with namespaces, I can't make much sense
of this patchset and it obfuscates the whole kobject thing more and
that's a bad direction to be heading toward.

> Looking through my code there are 3 types of callbacks.
> - Callbacks to the namespace type of a children.
>   .child_ns_type

Can you please also explain the relationships among kobjects, ns_types
and NSes?

> - Callbacks to find the namespace of a kobject.
>   .namespace
> - Callbacks on the a namespace type to find the namespace
>   of a particular context.
>   .current_ns
>   .initial_ns  (not used in my patchset)
>   .netlink_ns  (not used in my patchset)
> 
> In a world of weird explicitness I expect .child_ns_type and
> .namespace could be made to go away by pushing through explicit
> ns_type, and namespace parameters everywhere. But that seems
> like an awful lot of unnecessary code churn and bloat with
> the only real advantage being that we have an abstraction
> stored explicit at each layer.

* How much churn would it be?  I would be willing to trade quite a bit
  if the following can go away.  The sheer amount of indirection there
  scares me a lot.

  struct kobj_type {
  ...
	const struct kobj_ns_type_operations *(*child_ns_type)(struct kobject *kobj);
  ...
  };

* Is it necessary to teach kobject layer the concept of namespaces?
  Wouldn't it be possible to let kobject and sysfs deal with tags and
  make namespaces use them?

> static int kobj_bcast_filter(struct sock *dest_sk, struct sk_buff *skb, void *data)
> {
> 	struct kobject *kobj = data;
> 	const struct kobj_ns_type_operations *ops;
> 
> 	ops = kobj_ns_ops(kobj);
> 	if (ops) {
> 		const void *sock_ns, *ns;
> 		ns = kobj->ktype->namespace(kobj);
> 		sock_ns = ops->netlink_ns(dsk);
> 		return sock_ns != ns;
> 	}
> 
> 	return 0;
> }
> 
> initial_ns is used to figure out what the initial/default
> namespace is for a class of namespaces.  We only report
> with /sbin/hotplug events in the initial network namespace.
> At least for now.
> 
> static int kobj_usermode_filter(struct kobject *kobj)
> {
> 	const struct kobj_ns_type_operations *ops;
> 
> 	ops = kobj_ns_ops(kobj);
> 	if (ops) {
> 		const void *init_ns, *ns;
> 		ns = kobj->ktype->namespace(kobj);
> 		init_ns = ops->initial_ns();
> 		return ns != init_ns;
> 	}
> 
> 	return 0;
> }

I can understand you would need two different ways of establishing the
accessor depending on the mode of access (file IO or netlink) but can
initial_ns ever be dynamic?  Can't it just be void *inital_ns instead
of a callback?

Thanks.

-- 
tejun

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

Thread overview: 39+ 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-03-30 18:31 ` [PATCH 2/6] kobj: Add basic infrastructure for dealing with namespaces Eric W. Biederman
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 [this message]
2010-03-30 18:31 ` [PATCH 4/6] sysfs: Add support for tagged directories with untagged members Eric W. Biederman
2010-03-30 18:31 ` [PATCH 5/6] sysfs: Implement sysfs_delete_link Eric W. Biederman
2010-03-30 18:31 ` [PATCH 6/6] driver core: Implement ns directory support for device classes Eric W. Biederman
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-20 17:47 ` 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=4BB99C9C.4070308@gmail.com \
    --to=htejun@gmail.com \
    --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=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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).