2005-09-23 14:21:39

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH 3/3] fuse: check O_DIRECT

Check O_DIRECT and return -EINVAL error in open. dentry_open() also
checks this but only after the open method is called. This patch
optimizes away the unnecessary upcalls in this case.

Signed-off-by: Miklos Szeredi <[email protected]>

Index: linux/fs/fuse/file.c
===================================================================
--- linux.orig/fs/fuse/file.c 2005-09-21 11:55:45.000000000 +0200
+++ linux/fs/fuse/file.c 2005-09-23 15:24:23.000000000 +0200
@@ -23,6 +23,10 @@ int fuse_open_common(struct inode *inode
struct fuse_file *ff;
int err;

+ /* VFS checks this, but only _after_ ->open() */
+ if (file->f_flags & O_DIRECT)
+ return -EINVAL;
+
err = generic_file_open(inode, file);
if (err)
return err;


2005-09-23 21:27:56

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 3/3] fuse: check O_DIRECT

Miklos Szeredi <[email protected]> wrote:
>
> Check O_DIRECT and return -EINVAL error in open. dentry_open() also
> checks this but only after the open method is called. This patch
> optimizes away the unnecessary upcalls in this case.
>
> Signed-off-by: Miklos Szeredi <[email protected]>
>
> Index: linux/fs/fuse/file.c
> ===================================================================
> --- linux.orig/fs/fuse/file.c 2005-09-21 11:55:45.000000000 +0200
> +++ linux/fs/fuse/file.c 2005-09-23 15:24:23.000000000 +0200
> @@ -23,6 +23,10 @@ int fuse_open_common(struct inode *inode
> struct fuse_file *ff;
> int err;
>
> + /* VFS checks this, but only _after_ ->open() */
> + if (file->f_flags & O_DIRECT)
> + return -EINVAL;
> +
> err = generic_file_open(inode, file);
> if (err)
> return err;

This hardly seems worth optimising for?

2005-09-24 05:56:48

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 3/3] fuse: check O_DIRECT

> > + /* VFS checks this, but only _after_ ->open() */
> > + if (file->f_flags & O_DIRECT)
> > + return -EINVAL;
> > +
> > err = generic_file_open(inode, file);
> > if (err)
> > return err;
>
> This hardly seems worth optimising for?

It could be a correctness issue too: if filesystem has open() with
side effect, then it should fail before doing the open, not after.

Not a big deal, but I think it's worth the 3 lines.

Miklos