mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: viro@parcelfarce.linux.theplanet.co.uk
To: Linus Torvalds <torvalds@transmeta.com>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>,
	Linux FSdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	NFS maillist <nfs@lists.sourceforge.net>
Subject: Re: [PATCH 1/4] Optimize NFS open() calls by means of 'intents'...
Date: Fri, 23 May 2003 18:59:54 +0100	[thread overview]
Message-ID: <20030523175954.GD14406@parcelfarce.linux.theplanet.co.uk> (raw)
In-Reply-To: <Pine.LNX.4.44.0305230911160.21297-100000@home.transmeta.com>

On Fri, May 23, 2003 at 09:23:33AM -0700, Linus Torvalds wrote:
> 
> On Fri, 23 May 2003, Trond Myklebust wrote:
> > 
> > Minor cleanup of open() code. Put the original open flags, mode, etc. into
> > an 'opendata' structure that can be passed as an intent to lookup.
> 
> I don't mind the concepts, but I _really_ dislike the implementation.
> 
> For one thing, if you're creating a structure to pass in the flags for 
> open, then you should take the time to make the code _more_ readable 
> rather than less. In particular, the notion of having a structure like 
> this:
> 
> 	struct opendata {
> 		int flag;
> 		int mode;
> 		int acc_mode;
> 	};
> 
> where each of "flag" and "acc_mode" are magic bitfields just fills me with
> horror. 
> 
> So why not make those internal modes that we translate the "flags" into be 
> a real bitmap? That should make the code a lot more readable.
> 
> Also, I don't really understand why you want to have "opendata" and 
> "intent" as different structures. That's _especially_ true now that the 
> only intent is the "open" intent, but even if there were other intents, 
> I'd rather have something like this
> 
> 	struct lookup_info {
> 		enum type; /* open, validate, whatever.. */
> 		union {
> 			struct open_intent open;
> 			..
> 		} data;
> 	}
> 
> and gace tge flags (create/exclusive etc) inside that lookup_intent 
> instead of having multiple different pointers and transferring data from 
> one to the other at different phases of the "open".


Linus, that was one of the reasons why struct nameidata had been introduced
in the first place.  _And_ discussed with Peter, BTW, so I've no idea
where the hell does lookup_info come from.

Peter, Trond: please fold that stuff into struct nameidata (note that
flags are already there) and pass the pointer to it into methods.
That would have an extra benefit (also discussed before) of allowing to
bring credentials into the game - we could store them in the same place.

If we are up to changing method prototypes - let's do it properly.

  reply	other threads:[~2003-05-23 17:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-23 12:45 Trond Myklebust
2003-05-23 16:23 ` Linus Torvalds
2003-05-23 17:59   ` viro [this message]
2003-05-23 18:32     ` Andreas Dilger
2003-06-30 14:36 Trond Myklebust
2003-06-30 15:14 ` Matthew Wilcox
2003-07-01  9:39   ` Trond Myklebust
2003-06-30 14:37 Trond Myklebust
2003-06-30 14:39 ` Trond Myklebust

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=20030523175954.GD14406@parcelfarce.linux.theplanet.co.uk \
    --to=viro@parcelfarce.linux.theplanet.co.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nfs@lists.sourceforge.net \
    --cc=torvalds@transmeta.com \
    --cc=trond.myklebust@fys.uio.no \
    /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

all inboxes | Powered by JetHome®