linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Dave Chinner <david@fromorbit.com>
Cc: Jan Kara <jack@suse.cz>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	"Serge E. Hallyn" <serge@hallyn.com>,
	David Miller <davem@davemloft.net>,
	Steven Whitehouse <swhiteho@redhat.com>,
	Mark Fasheh <mfasheh@suse.com>, Joel Becker <jlbec@evilplan.org>,
	Ben Myers <bpm@sgi.com>, Alex Elder <elder@kernel.org>,
	Dmitry Monakhov <dmonakhov@openvz.org>,
	Abhijith Das <adas@redhat.com>
Subject: Re: [PATCH] userns: Add basic quota support v4
Date: Wed, 19 Sep 2012 18:28:06 -0700	[thread overview]
Message-ID: <87vcf91o0p.fsf@xmission.com> (raw)
In-Reply-To: <20120831011758.GH15292@dastard> (Dave Chinner's message of "Fri, 31 Aug 2012 11:17:58 +1000")


Dave thank you earlier taking some time to do review.  It made me
realize that my code was not as mature as it needed to be.

That said you missed a lot of important details, and I will aim at
to address some of the highlights.

Precedent in naming. <uidgid.h> that has been merged since 3.5, earlier
there is make_pte.   Much of your review sounds like you are unhappy
with the general infrastructure that was built for user namespaces,
and already merged.  Short of an overwhelmingly better suggestion
at this point I am not going to revisit naming.

Passing structures by value is a core part of the design and a necessity
for strong type safety in C, and again already in common use in the
kernel.

Using BUG on code paths that can not be triggered, simply means that
in that rare and unlikely event there is a clear trace to follow,
and xfs already has plenty of invocations of BUG, some of which
I managed to trigger with the xfs test suite.  So I can not find
much sense in your arguments about being clear that something is a BUG.

Your comments on where xfs uses projid are just laughable.

>> So I am inclined to rename this field projid, but I don't know if it is
>> worth respinning the patch for something so trivial.
>
> I'd like to think you are joking, given that the patch series is all
> about using consistent, verifiable identifiers... :/

The patch series is all about implementing the user namespace.

To make it hard to miss places where conversions need to between the
internal kernel form and the form stored in filesystems or the form that
userspace uses I use types that are not assignment compatible.  Pushing
the types deeper in the code keeps me honest and let's me find cases
I was not expecting like the ioctl interface of xfs that largely
duplicates much of the vfs functionality.

Eric

  parent reply	other threads:[~2012-09-20  1:28 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-25 23:54 [REVIEW][PATCH 0/15] userns subsystem conversions Eric W. Biederman
2012-08-25 23:58 ` [REVIEW][PATCH 01/15] userns: Enable building of pf_key sockets when user namespace support is enabled Eric W. Biederman
2012-08-25 23:59 ` [REVIEW][PATCH 02/15] userns: Make credential debugging user namespace safe Eric W. Biederman
2012-08-25 23:59 ` [REVIEW][PATCH 03/15] userns: Convert security/keys to the new userns infrastructure Eric W. Biederman
2012-08-26  0:00 ` [REVIEW][PATCH 04/15] userns: net: Call key_alloc with GLOBAL_ROOT_UID, GLOBAL_ROOT_GID instead of 0, 0 Eric W. Biederman
2012-08-26  0:00 ` [REVIEW][PATCH 05/15] userns: Convert ipc to use kuid and kgid where appropriate Eric W. Biederman
2012-08-26  0:01 ` [REVIEW][PATCH 07/15] userns: Convert taskstats to handle the user and pid namespaces Eric W. Biederman
2012-08-26  0:02 ` [REVIEW][PATCH 09/15] userns: Convert process event connector to handle kuids and kgids Eric W. Biederman
2012-08-26 12:33   ` Evgeniy Polyakov
2012-08-26 13:43     ` Eric W. Biederman
2012-08-26  0:03 ` [REVIEW][PATCH 10/15] userns: Convert debugfs to use kuid/kgid where appropriate Eric W. Biederman
2012-09-05 21:09   ` Greg Kroah-Hartman
2012-08-26  0:04 ` [REVIEW][PATCH 11/15] userns: Teach trace to use from_kuid Eric W. Biederman
2012-08-26  0:18   ` Steven Rostedt
2012-08-26  0:28     ` Eric W. Biederman
2012-08-26  0:05 ` [REVIEW][PATCH 12/15] userns: Convert drm to use kuid and kgid and struct pid where appropriate Eric W. Biederman
2012-09-13  1:31   ` Dave Airlie
2012-09-13  2:14     ` Eric W. Biederman
2012-09-13  3:29       ` Dave Airlie
2012-08-26  0:07 ` [REVIEW][PATCH 15/15] userns: Convert configfs to use kuid and kgid " Eric W. Biederman
2012-08-26 13:00 ` [PATCH 06/15] userns: Convert audit " Eric W. Biederman
     [not found] ` <9E0E8AAC-9548-4009-AE29-D368244D8EEA@dubeyko.com>
2012-08-26 14:25   ` [REVIEW][PATCH 0/15] userns subsystem conversions Eric W. Biederman
     [not found] ` <87harqecvk.fsf@xmission.com>
2012-08-27  8:50   ` [REVIEW][PATCH 13/15] userns: Add basic quota support Jan Kara
2012-08-27 15:54     ` Eric W. Biederman
2012-08-28  0:12     ` [PATCH] userns: Add basic quota support v2 Eric W. Biederman
2012-08-28  9:05       ` Jan Kara
2012-08-28  9:44         ` Boaz Harrosh
2012-08-28 17:34         ` Eric W. Biederman
2012-08-28 17:36           ` [PATCH] userns: Add basic quota support v3 Eric W. Biederman
2012-08-28 17:51           ` [PATCH] userns: Add basic quota support v2 Jan Kara
2012-08-28 19:09             ` [PATCH] userns: Add basic quota support v4 Eric W. Biederman
2012-08-29  2:10               ` Dave Chinner
2012-08-29  9:31                 ` Eric W. Biederman
2012-08-31  1:17                   ` Dave Chinner
2012-09-05  5:20                     ` Eric W. Biederman
2012-09-20  1:28                     ` Eric W. Biederman [this message]
2012-08-27  8:58   ` [REVIEW][PATCH 13/15] userns: Add basic quota support Steven Whitehouse

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=87vcf91o0p.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=adas@redhat.com \
    --cc=bpm@sgi.com \
    --cc=davem@davemloft.net \
    --cc=david@fromorbit.com \
    --cc=dmonakhov@openvz.org \
    --cc=elder@kernel.org \
    --cc=jack@suse.cz \
    --cc=jlbec@evilplan.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mfasheh@suse.com \
    --cc=netdev@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=swhiteho@redhat.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).