Re: [RFC] atomic open(..., O_CREAT | ...)

[Miklos Szeredi <miklos@szeredi.hu>]

> Intents are meant as optimisations, not replacements for existing
> operations. I'm therefore not really comfortable about having them
> return errors at all.

In my case they are not an optimization, rather the only way to
correctly perform an open with O_CREAT.

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

If you use the nameidata after path_release_open_intent(), you're
screwed anyway, since nd->mnt and nd->dentry have already been
released.

If there's any chance that the path walk restart thing will invoke the
filesystems open code twice (I doubt it), then the filesystem must
make sure to check intent.open.file, whether it has already been set,
and fput() it before setting it another time.

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

I have absolutely no idea. 

Looking closer, there's a problem with O_TRUNC as well:

	namei_flags = flags;
	if ((namei_flags+1) & O_ACCMODE)
		namei_flags++;
	if (namei_flags & O_TRUNC)
		namei_flags |= 2;

So if flags is O_RDONLY|O_TRUNC, intent.open.flags will be
FMODE_WRITE|FMODE_READ|O_TRUNC, but filp->f_mode will be FMODE_READ.

This is also undocumented (or rather documented to be undefined), but
the behavior is perfectly logical, and I can imagine some application
relying on it.

Miklos


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/