All of lore.kernel.org
 help / color / mirror / Atom feed
From: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lxc-devel-cunTk1MwBs9qMoObBWhMNEqPaTDuhLve2LY78lusg7I@public.gmane.org,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
	hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org,
	"Serge E. Hallyn"
	<serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Subject: Re: [PATCH 1/8] kernfs: Add API to generate relative kernfs path
Date: Wed, 9 Dec 2015 22:13:27 +0000	[thread overview]
Message-ID: <20151209221327.GA13029@ubuntumail> (raw)
In-Reply-To: <20151209213806.GP30240-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>

Quoting Tejun Heo (tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org):
> Hello, Serge.
> 
> On Wed, Dec 09, 2015 at 01:28:54PM -0600, serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org wrote:
> > +/* kernfs_node_depth - compute depth from @from to @to */
> > +static size_t kernfs_depth(struct kernfs_node *from, struct kernfs_node *to)
> ...
> > +char *kernfs_path(struct kernfs_node *kn, char *buf, size_t buflen)
> > +{
> > +	return kernfs_path_from_node(NULL, kn, buf, buflen);
> > +}
> ...
> > diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
> > index 5d4e9c4..d025ebd 100644
> > --- a/include/linux/kernfs.h
> > +++ b/include/linux/kernfs.h
> > @@ -267,6 +267,9 @@ static inline bool kernfs_ns_enabled(struct kernfs_node *kn)
> >  
> >  int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen);
> >  size_t kernfs_path_len(struct kernfs_node *kn);
> > +char * __must_check kernfs_path_from_node(struct kernfs_node *root_kn,
> > +					  struct kernfs_node *kn, char *buf,
> > +					  size_t buflen);
> 
> I think I commented on the same thing before, but I think it'd make
> more sense to put @from after @to

Oh.  You said that for kernfs_path_from_node_locked(), and those were
changed.  kernfs_path_form_node() is a different fn, but

> and the prototype is using @root_kn
> which is a bit confusing.

we can rename kn_root to from here if you think that's clearer (and
change the order here as well).

> Was converting the path functions to return
> length too much work?  If so, that's fine but please explain what
> decisions were made.

Yes, I had replied saying:

 |I can change that, but the callers right now don't re-try with
 |larger buffer anyway, so this would actually complicate them just
 |a smidgeon.  Would you want them changed to do that?  (pr_cont_kernfs_path
 |right now writes into a static char[] for instance)

I can still make that change if you like.

> I skimmed through the series and spotted several other review points
> which didn't get addressed.  Can you please go over the previous
> review cycle and address the review points?

I did go through every email twice, once while making changes (one
branch per response) and once while making changelog for each patch,
sorry about whatever I missed.  I'll go through each again.

I'm going to be out for awhile after today, so next version will
unfortunately take awhile.

thanks,
-serge

WARNING: multiple messages have this Message-ID (diff)
From: Serge Hallyn <serge.hallyn@ubuntu.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org, adityakali@google.com,
	linux-api@vger.kernel.org, containers@lists.linux-foundation.org,
	cgroups@vger.kernel.org, lxc-devel@lists.linuxcontainers.org,
	akpm@linux-foundation.org, ebiederm@xmission.com,
	gregkh@linuxfoundation.org, lizefan@huawei.com,
	hannes@cmpxchg.org,
	"Serge E. Hallyn" <serge.hallyn@canonical.com>
Subject: Re: [PATCH 1/8] kernfs: Add API to generate relative kernfs path
Date: Wed, 9 Dec 2015 22:13:27 +0000	[thread overview]
Message-ID: <20151209221327.GA13029@ubuntumail> (raw)
In-Reply-To: <20151209213806.GP30240@mtj.duckdns.org>

Quoting Tejun Heo (tj@kernel.org):
> Hello, Serge.
> 
> On Wed, Dec 09, 2015 at 01:28:54PM -0600, serge.hallyn@ubuntu.com wrote:
> > +/* kernfs_node_depth - compute depth from @from to @to */
> > +static size_t kernfs_depth(struct kernfs_node *from, struct kernfs_node *to)
> ...
> > +char *kernfs_path(struct kernfs_node *kn, char *buf, size_t buflen)
> > +{
> > +	return kernfs_path_from_node(NULL, kn, buf, buflen);
> > +}
> ...
> > diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
> > index 5d4e9c4..d025ebd 100644
> > --- a/include/linux/kernfs.h
> > +++ b/include/linux/kernfs.h
> > @@ -267,6 +267,9 @@ static inline bool kernfs_ns_enabled(struct kernfs_node *kn)
> >  
> >  int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen);
> >  size_t kernfs_path_len(struct kernfs_node *kn);
> > +char * __must_check kernfs_path_from_node(struct kernfs_node *root_kn,
> > +					  struct kernfs_node *kn, char *buf,
> > +					  size_t buflen);
> 
> I think I commented on the same thing before, but I think it'd make
> more sense to put @from after @to

Oh.  You said that for kernfs_path_from_node_locked(), and those were
changed.  kernfs_path_form_node() is a different fn, but

> and the prototype is using @root_kn
> which is a bit confusing.

we can rename kn_root to from here if you think that's clearer (and
change the order here as well).

> Was converting the path functions to return
> length too much work?  If so, that's fine but please explain what
> decisions were made.

Yes, I had replied saying:

 |I can change that, but the callers right now don't re-try with
 |larger buffer anyway, so this would actually complicate them just
 |a smidgeon.  Would you want them changed to do that?  (pr_cont_kernfs_path
 |right now writes into a static char[] for instance)

I can still make that change if you like.

> I skimmed through the series and spotted several other review points
> which didn't get addressed.  Can you please go over the previous
> review cycle and address the review points?

I did go through every email twice, once while making changes (one
branch per response) and once while making changelog for each patch,
sorry about whatever I missed.  I'll go through each again.

I'm going to be out for awhile after today, so next version will
unfortunately take awhile.

thanks,
-serge

  parent reply	other threads:[~2015-12-09 22:13 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-09 19:28 CGroup Namespaces (v7) serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28 ` serge.hallyn
2015-12-09 19:28 ` [PATCH 1/8] kernfs: Add API to generate relative kernfs path serge.hallyn
     [not found]   ` <1449689341-28742-2-git-send-email-serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
2015-12-09 21:38     ` Tejun Heo
2015-12-09 21:38       ` Tejun Heo
     [not found]       ` <20151209213806.GP30240-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-12-09 22:13         ` Serge Hallyn [this message]
2015-12-09 22:13           ` Serge Hallyn
2015-12-09 22:36           ` Tejun Heo
2015-12-09 22:36             ` Tejun Heo
2015-12-09 22:51             ` Serge E. Hallyn
     [not found]             ` <20151209223651.GQ30240-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-12-09 22:51               ` Serge E. Hallyn
2015-12-10  1:28               ` Serge E. Hallyn
2015-12-10  1:28                 ` Serge E. Hallyn
2015-12-09 22:36           ` Tejun Heo
2015-12-09 22:13         ` Serge Hallyn
2015-12-09 21:38     ` Tejun Heo
     [not found] ` <1449689341-28742-1-git-send-email-serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
2015-12-09 19:28   ` serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28   ` [PATCH 2/8] sched: new clone flag CLONE_NEWCGROUP for cgroup namespace serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28     ` serge.hallyn
2015-12-09 19:28   ` [PATCH 3/8] cgroup: introduce cgroup namespaces serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28     ` serge.hallyn
2015-12-09 19:28   ` [PATCH 4/8] cgroup: cgroup namespace setns support serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28   ` serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28     ` serge.hallyn
2015-12-09 19:28   ` [PATCH 5/8] kernfs: define kernfs_node_dentry serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28     ` serge.hallyn
2015-12-09 19:28   ` [PATCH 6/8] cgroup: mount cgroupns-root when inside non-init cgroupns serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:28     ` serge.hallyn
2015-12-09 19:29   ` [PATCH 7/8] cgroup: Add documentation for cgroup namespaces serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:29   ` [PATCH 8/8] Add FS_USERNS_FLAG to cgroup fs serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:29     ` serge.hallyn
2015-12-09 19:29   ` serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-09 19:29 ` [PATCH 7/8] cgroup: Add documentation for cgroup namespaces serge.hallyn
  -- strict thread matches above, loose matches on Subject: below --
2016-01-29  8:54 CGroup Namespaces (v10) serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
     [not found] ` <1454057651-23959-1-git-send-email-serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
2016-01-29  8:54   ` [PATCH 1/8] kernfs: Add API to generate relative kernfs path serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2016-01-29  8:54   ` serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2016-01-29  8:54     ` serge.hallyn
2016-01-04 19:54 CGroup Namespaces (v9) serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
     [not found] ` <1451937294-22589-1-git-send-email-serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
2016-01-04 19:54   ` [PATCH 1/8] kernfs: Add API to generate relative kernfs path serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2016-01-04 19:54     ` serge.hallyn
2015-12-23  4:23 CGroup Namespaces (v8) serge.hallyn
     [not found] ` <1450844609-9194-1-git-send-email-serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
2015-12-23  4:23   ` [PATCH 1/8] kernfs: Add API to generate relative kernfs path serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA
2015-12-23  4:23     ` serge.hallyn
     [not found]     ` <1450844609-9194-2-git-send-email-serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
2015-12-23 16:08       ` Tejun Heo
2015-12-23 16:08       ` Tejun Heo
2015-12-23 16:08         ` Tejun Heo
     [not found]         ` <20151223160854.GF5003-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-12-23 16:36           ` Serge E. Hallyn
2015-12-23 16:36             ` Serge E. Hallyn
2015-12-23 16:36           ` Serge E. Hallyn
2015-12-23 16:24       ` Tejun Heo
2015-12-23 16:24         ` Tejun Heo
     [not found]         ` <20151223162433.GH5003-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-12-23 16:51           ` Greg KH
2015-12-23 16:51             ` Greg KH
2015-12-23 16:51           ` Greg KH
2015-12-23 16:24       ` Tejun Heo
2015-11-16 19:51 CGroup Namespaces (v4) serge-A9i7LUbDfNHQT0dZR+AlfA
     [not found] ` <1447703505-29672-1-git-send-email-serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>
2015-11-16 19:51   ` [PATCH 1/8] kernfs: Add API to generate relative kernfs path serge-A9i7LUbDfNHQT0dZR+AlfA
2015-11-16 19:51     ` serge
     [not found]     ` <1447703505-29672-2-git-send-email-serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>
2015-11-24 16:16       ` Tejun Heo
2015-11-24 16:16       ` Tejun Heo
2015-11-24 16:16         ` Tejun Heo
     [not found]         ` <20151124161630.GL17033-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-11-24 16:17           ` Tejun Heo
2015-11-24 16:17             ` Tejun Heo
     [not found]             ` <20151124161709.GM17033-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-11-24 17:43               ` Serge E. Hallyn
2015-11-24 17:43                 ` Serge E. Hallyn
2015-11-24 17:43               ` Serge E. Hallyn
2015-11-24 16:17           ` Tejun Heo
2015-11-27  5:25           ` Serge E. Hallyn
2015-11-27  5:25           ` Serge E. Hallyn
2015-11-27  5:25             ` Serge E. Hallyn
     [not found]             ` <20151127052511.GA25490-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
2015-11-30 15:11               ` Tejun Heo
2015-11-30 15:11               ` Tejun Heo
2015-11-30 15:11                 ` Tejun Heo
     [not found]                 ` <20151130151147.GG3535-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-11-30 18:37                   ` Serge E. Hallyn
2015-11-30 18:37                   ` Serge E. Hallyn
2015-11-30 18:37                     ` Serge E. Hallyn
     [not found]                     ` <20151130183758.GA25433-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
2015-11-30 22:53                       ` Tejun Heo
2015-11-30 22:53                     ` Tejun Heo
     [not found]                       ` <20151130225318.GD9039-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
2015-12-01  2:08                         ` Serge E. Hallyn
2015-12-01  2:08                         ` Serge E. Hallyn
2015-12-01  2:08                           ` Serge E. Hallyn

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=20151209221327.GA13029@ubuntumail \
    --to=serge.hallyn-gewih/nmzzlqt0dzr+alfa@public.gmane.org \
    --cc=adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \
    --cc=lxc-devel-cunTk1MwBs9qMoObBWhMNEqPaTDuhLve2LY78lusg7I@public.gmane.org \
    --cc=serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org \
    --cc=tj-DgEjT+Ai2ygdnm+yROfE0A@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.