* [PATCH] overlay: Plumb through flush method @ 2020-11-30 3:00 Sargun Dhillon 2020-12-03 10:16 ` Sargun Dhillon 0 siblings, 1 reply; 6+ messages in thread From: Sargun Dhillon @ 2020-11-30 3:00 UTC (permalink / raw) Cc: Sargun Dhillon, linux-fsdevel, linux-unionfs, Miklos Szeredi, Amir Goldstein 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 <sargun@sargun.me> 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] overlay: Plumb through flush method 2020-11-30 3:00 [PATCH] overlay: Plumb through flush method Sargun Dhillon @ 2020-12-03 10:16 ` Sargun Dhillon 2020-12-03 10:32 ` Amir Goldstein 0 siblings, 1 reply; 6+ messages in thread From: Sargun Dhillon @ 2020-12-03 10:16 UTC (permalink / raw) To: Miklos Szeredi, Amir Goldstein; +Cc: Linux FS-devel Mailing List, overlayfs On Sun, Nov 29, 2020 at 7:00 PM Sargun Dhillon <sargun@sargun.me> wrote: > > 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 <sargun@sargun.me> > 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. It seems like it would be useful if anyone uses NFS, or CIFS as an upperdir. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] overlay: Plumb through flush method 2020-12-03 10:16 ` Sargun Dhillon @ 2020-12-03 10:32 ` Amir Goldstein 2020-12-03 18:32 ` Sargun Dhillon 0 siblings, 1 reply; 6+ messages in thread From: Amir Goldstein @ 2020-12-03 10:32 UTC (permalink / raw) To: Sargun Dhillon; +Cc: Miklos Szeredi, Linux FS-devel Mailing List, overlayfs On Thu, Dec 3, 2020 at 12:16 PM Sargun Dhillon <sargun@sargun.me> wrote: > > On Sun, Nov 29, 2020 at 7:00 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > 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 <sargun@sargun.me> > > 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? > > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] overlay: Plumb through flush method 2020-12-03 10:32 ` Amir Goldstein @ 2020-12-03 18:32 ` Sargun Dhillon 2021-01-20 15:24 ` Miklos Szeredi 0 siblings, 1 reply; 6+ messages in thread From: Sargun Dhillon @ 2020-12-03 18:32 UTC (permalink / raw) To: Amir Goldstein; +Cc: Miklos Szeredi, Linux FS-devel Mailing List, overlayfs On Thu, Dec 3, 2020 at 2:32 AM Amir Goldstein <amir73il@gmail.com> wrote: > > On Thu, Dec 3, 2020 at 12:16 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > On Sun, Nov 29, 2020 at 7:00 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > > > 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 <sargun@sargun.me> > > > 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? > > > > > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] overlay: Plumb through flush method 2020-12-03 18:32 ` Sargun Dhillon @ 2021-01-20 15:24 ` Miklos Szeredi 2021-01-23 12:54 ` Amir Goldstein 0 siblings, 1 reply; 6+ messages in thread From: Miklos Szeredi @ 2021-01-20 15:24 UTC (permalink / raw) To: Sargun Dhillon; +Cc: Amir Goldstein, Linux FS-devel Mailing List, overlayfs On Thu, Dec 3, 2020 at 7:32 PM Sargun Dhillon <sargun@sargun.me> wrote: > > On Thu, Dec 3, 2020 at 2:32 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Thu, Dec 3, 2020 at 12:16 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > > > On Sun, Nov 29, 2020 at 7:00 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > > > > > 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 <sargun@sargun.me> > > > > 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? > > > > > > > > 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). Thanks, Miklos ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] overlay: Plumb through flush method 2021-01-20 15:24 ` Miklos Szeredi @ 2021-01-23 12:54 ` Amir Goldstein 0 siblings, 0 replies; 6+ messages in thread From: Amir Goldstein @ 2021-01-23 12:54 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Sargun Dhillon, Linux FS-devel Mailing List, overlayfs On Wed, Jan 20, 2021 at 5:24 PM Miklos Szeredi <miklos@szeredi.hu> wrote: > > On Thu, Dec 3, 2020 at 7:32 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > On Thu, Dec 3, 2020 at 2:32 AM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > On Thu, Dec 3, 2020 at 12:16 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > > > > > On Sun, Nov 29, 2020 at 7:00 PM Sargun Dhillon <sargun@sargun.me> wrote: > > > > > > > > > > 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 <sargun@sargun.me> > > > > > 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? > > > > > > > > > > > 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; } ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-23 12:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-30 3:00 [PATCH] overlay: Plumb through flush method Sargun Dhillon 2020-12-03 10:16 ` Sargun Dhillon 2020-12-03 10:32 ` Amir Goldstein 2020-12-03 18:32 ` Sargun Dhillon 2021-01-20 15:24 ` Miklos Szeredi 2021-01-23 12:54 ` Amir Goldstein
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).