On Wed, Jan 20, 2021 at 5:24 PM Miklos Szeredi [off-list ref] wrote:

On Thu, Dec 3, 2020 at 7:32 PM Sargun Dhillon [off-list ref] wrote:

quoted

On Thu, Dec 3, 2020 at 2:32 AM Amir Goldstein [off-list ref] wrote:

quoted

On Thu, Dec 3, 2020 at 12:16 PM Sargun Dhillon [off-list ref] wrote:

quoted

On Sun, Nov 29, 2020 at 7:00 PM Sargun Dhillon [off-list ref] wrote:

quoted

Filesystems can implement their own flush method that release
resources, or manipulate caches. Currently if one of these
filesystems is used with overlayfs, the flush method is not called.

Signed-off-by: Sargun Dhillon <redacted>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-unionfs@vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/file.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index efccb7c1f9bc..802259f33c28 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c

@@ -787,6 +787,16 @@ static loff_t ovl_remap_file_range(struct file *file_in, loff_t pos_in,
                            remap_flags, op);
 }

+static int ovl_flush(struct file *file, fl_owner_t id)
+{
+       struct file *realfile = file->private_data;
+
+       if (realfile->f_op->flush)
+               return realfile->f_op->flush(realfile, id);
+
+       return 0;
+}
+
 const struct file_operations ovl_file_operations = {
        .open           = ovl_open,
        .release        = ovl_release,

@@ -798,6 +808,7 @@ const struct file_operations ovl_file_operations = {
        .fallocate      = ovl_fallocate,
        .fadvise        = ovl_fadvise,
        .unlocked_ioctl = ovl_ioctl,
+       .flush          = ovl_flush,
 #ifdef CONFIG_COMPAT
        .compat_ioctl   = ovl_compat_ioctl,
 #endif
--

2.25.1

Amir, Miklos,
Is this acceptable? I discovered this being a problem when we had the discussion
of whether the volatile fs should return an error on close on dirty files.

Yes, looks ok.
Maybe we want to check if the realfile is upper although
maybe flush can release resources also on read only fs?

quoted

It seems like it would be useful if anyone uses NFS, or CIFS as an upperdir.

They are not supported as upperdir. only FUSE is.

Thanks,
Amir.

VFS does it on read-only files / mounts, so we should probably do the
same thing.

Right, but it should handle files copied up after the oipen (i.e. call
ovl_real_fdget() to get the real file).

Applied patch is missing fdput() xfstests fail.
Following tested fix.

Thanks,
Amir.

--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c

@@ -693,12 +693,17 @@ static int ovl_flush(struct file *file, fl_owner_t id)
        int err;

        err = ovl_real_fdget(file, &real);
-       if (!err && real.file->f_op->flush) {
+       if (err)
+               return err;
+
+       if (real.file->f_op->flush) {
                old_cred = ovl_override_creds(file_inode(file)->i_sb);
                err = real.file->f_op->flush(real.file, id);
                revert_creds(old_cred);
        }

+       fdput(real);
+
        return err;
 }