On 5/19/22 22:09, Vivek Goyal wrote:
On Thu, May 19, 2022 at 04:43:58PM +0200, Miklos Szeredi wrote:Hi Al, Do you see anything bad with allowing ->atomic_open() to take a positive dentry and possibly invalidate it after it does the atomic LOOKUP/CREATE+OPEN? It looks wrong not to allow optimizing away the roundtrip associated with revalidation when we do allow optimizing away the roundtrip for the initial lookup in the same situation. Thanks, Miklos diff --git a/fs/namei.c b/fs/namei.c index 509657fdf4f5..d35b5cbf7f64 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3267,7 +3267,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, dput(dentry); dentry = NULL; } - if (dentry->d_inode) { + if (dentry->d_inode && !d_atomic_open(dentry)) { /* Cached positive dentry: will open in f_op->open */ return dentry;Hi Miklos, I see that lookup_open() calls d_revalidate() first. So basically idea is that fuse ->.d_revalidate will skip LOOKUP needed to make sure dentry is still valid (Only if atomic lookup+open is implemented) and return 1 claiming dentry is valid. And later in ->atomic_open(), it will either open the file or get an error and invalidate dentry. Hence will save one LOOKUP in success case. Do I understand the intent right?
Yeah, I think Dharmendra and I had internally already debated over this. In order to reduce complexity for the patches we preferred to go without vfs modifications.
I assume the patch is a follow up to this comment https://lore.kernel.org/all/20220517100744.26849-1-dharamhans87@xxxxxxxxx/T/#m8bd440ddea4c135688c829f34e93371e861ba9fa Thanks, Bernd
