ty den 09.08.2005 Klokka 09:46 (+0200) skreiv Miklos Szeredi: > > We've already got a patch that does this, and that I'm queueing up for > > inclusion. > > Cool! > > > http://client.linux-nfs.org/Linux-2.6.x/2.6.12/linux-2.6.12-63-open_file_intents.dif > > Comments: > > > /* > > + * Open intents have to release any file pointer that was allocated > > + * but not used by the VFS. > > + */ > > +void path_release_open_intent(struct nameidata *nd) > > +{ > > + if ((nd->flags & LOOKUP_OPEN) && nd->intent.open.file != NULL) { > > + fput(nd->intent.open.file); > > I think you should consider adding this: > > + if (!IS_ERR(nd->intent.open.file)) > + fput(nd->intent.open.file); > > so the filesystem can delay returning the error from the open > operation until the other errors have been sorted out by the lookup > code.
Intents are meant as optimisations, not replacements for existing operations. I'm therefore not really comfortable about having them return errors at all. > > + nd->intent.open.file = NULL; > > Why is this NULL assignment needed? nd will not be used after this. > > > + } > > + path_release(nd); > > +} > > + > > It could be dropped. The reason for putting it in is that some parts of the VFS may restart a path walk operation if it fails (see for instance the ESTALE handling). > > As for the "orig flags" thing. What is the point of that? > > dentry_open() needs the original open flags, not the transformed ones > stored in intent.open.flags. > > The behavior is slightly strange, since filp_open() calculates > namei_flags (which gets stored in intent.open.flags) so that an > O_ACCMODE of 3 is transformed into FMODE_READ | FMODE_WRITE. > > But dentry_open() calculates filp->f_mode, so that O_ACCMODE of 3 is > transformed into zero. > > This means that the (undocumented) access mode of 3 will require > read-write permission, but will allow neither read() nor write() on > the opened file. > > If we want to keep this behavior, then the orig_flags field is needed. Why do we want to keep this behaviour? It is undocumented, it is non-posix, and it appears to do nothing you cannot do with the existing access() call. Are there any applications using it? If so, which ones, and why? Cheers, Trond - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
