Re: [PATCH 1/4] Optimize NFS open() calls by means of 'intents'...

[Andreas Dilger]

On May 23, 2003  18:59 +0100, [e-mail] wrote:
> 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.

Yes, that is where Lustre puts the intent data already in 2.5.  The code
that Trond submitted is AFAICS totally different than what we have been
using for Lustre, but I don't know if Trond and Peter have been discussing
this in private or not.  I've CC'd Peter on this email, although I imagine
he is already getting it by way of fsdevel - he's just in a different
timezone right now.

> 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.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/