Re: fcntl(fd, F_SETFL, O_DIRECT) succeeds followed by EINVAL in write

[Daniel Black <daniel@mariadb.org>]

On Wed, Jan 26, 2022 at 2:02 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jan 26, 2022 at 09:05:48AM +1100, Daniel Black wrote:
> > The kernel code in setfl seems to want to return EINVAL for
> > filesystems without a direct_IO structure member assigned,
> >
> > A noop_direct_IO seems to be used frequently to just return EINVAL
> > (like cifs_direct_io).
>
> Sorry for the confusion.  You've caught us mid-transition.  Eventually,
> ->direct_IO will be deleted, but for now it signifies whether or not the
> filesystem supports O_DIRECT, even though it's not used (except in some
> scenarios you don't care about).

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 9c6c6a3e2de5..ff55a904bb4e 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -58,7 +58,8 @@ static int setfl(int fd, struct file * filp,
unsigned long arg)
        /* Pipe packetized mode is controlled by O_DIRECT flag */
        if (!S_ISFIFO(inode->i_mode) && (arg & O_DIRECT)) {
                if (!filp->f_mapping || !filp->f_mapping->a_ops ||
-                       !filp->f_mapping->a_ops->direct_IO)
+                       !filp->f_mapping->a_ops->direct_IO ||
+                       filp->f_mapping->a_ops->direct_IO == noop_direct_IO)
                                return -EINVAL;
        }

The above patch prevents:

  filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags & ~SETFL_MASK);

being executed at the bottom of setfl which keeps the file descriptor
out of O_DIRECT mode when
the filesystem (like CIFS doesn't support it). In the original strace

So while you are mid-transition, the relatively simple flag of
direct_IO is good enough a protection
for a file descriptor entering a mode that isn't supported.

Is this an acceptable stop gap concept and/or stable backport?

>.., but I'm not quite sure what limitations cifs imposes.

Given cifs_direct_io is equivalent to the noop_direct_IO return
-EINVAL now, there's no direct io
there as I discovered testing the bug report
https://jira.mariadb.org/browse/MDEV-26970.

My patch two of the series would be to replace cifs_direct_io with
noop_direct_IO.