| Message ID | 20180412150826.20988-14-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 12, 2018 at 05:08:04PM +0200, Miklos Szeredi wrote: > Implement stacked fsync(). > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > fs/overlayfs/file.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c > index b98204c1c19c..4417527667ff 100644 > --- a/fs/overlayfs/file.c > +++ b/fs/overlayfs/file.c > @@ -222,10 +222,30 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) > return ret; > } > > +static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) > +{ > + struct fd real; > + const struct cred *old_cred; > + int ret; > + > + ret = ovl_real_file(file, &real); > + if (ret) > + return ret; > + > + old_cred = ovl_override_creds(file_inode(file)->i_sb); > + ret = vfs_fsync_range(real.file, start, end, datasync); > + revert_creds(old_cred); Can we avoid calling fsync() on real file if it is not upper. Is it worth optimizing. Vivek > + > + fdput(real); > + > + return ret; > +} > + > const struct file_operations ovl_file_operations = { > .open = ovl_open, > .release = ovl_release, > .llseek = ovl_llseek, > .read_iter = ovl_read_iter, > .write_iter = ovl_write_iter, > + .fsync = ovl_fsync, > }; > -- > 2.14.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 23, 2018 at 3:36 PM, Vivek Goyal <vgoyal@redhat.com> wrote: > On Thu, Apr 12, 2018 at 05:08:04PM +0200, Miklos Szeredi wrote: >> Implement stacked fsync(). >> >> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> >> --- >> fs/overlayfs/file.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c >> index b98204c1c19c..4417527667ff 100644 >> --- a/fs/overlayfs/file.c >> +++ b/fs/overlayfs/file.c >> @@ -222,10 +222,30 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) >> return ret; >> } >> >> +static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) >> +{ >> + struct fd real; >> + const struct cred *old_cred; >> + int ret; >> + >> + ret = ovl_real_file(file, &real); >> + if (ret) >> + return ret; >> + >> + old_cred = ovl_override_creds(file_inode(file)->i_sb); >> + ret = vfs_fsync_range(real.file, start, end, datasync); >> + revert_creds(old_cred); > > Can we avoid calling fsync() on real file if it is not upper. Is it worth > optimizing. Not sure it's worth bothering with. If caller of fsync(2) didn't worry about cost, then why should we? Thanks, Miklos
On Mon, Apr 23, 2018 at 03:39:45PM +0200, Miklos Szeredi wrote: > On Mon, Apr 23, 2018 at 3:36 PM, Vivek Goyal <vgoyal@redhat.com> wrote: > > On Thu, Apr 12, 2018 at 05:08:04PM +0200, Miklos Szeredi wrote: > >> Implement stacked fsync(). > >> > >> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > >> --- > >> fs/overlayfs/file.c | 20 ++++++++++++++++++++ > >> 1 file changed, 20 insertions(+) > >> > >> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c > >> index b98204c1c19c..4417527667ff 100644 > >> --- a/fs/overlayfs/file.c > >> +++ b/fs/overlayfs/file.c > >> @@ -222,10 +222,30 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) > >> return ret; > >> } > >> > >> +static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) > >> +{ > >> + struct fd real; > >> + const struct cred *old_cred; > >> + int ret; > >> + > >> + ret = ovl_real_file(file, &real); > >> + if (ret) > >> + return ret; > >> + > >> + old_cred = ovl_override_creds(file_inode(file)->i_sb); > >> + ret = vfs_fsync_range(real.file, start, end, datasync); > >> + revert_creds(old_cred); > > > > Can we avoid calling fsync() on real file if it is not upper. Is it worth > > optimizing. > > Not sure it's worth bothering with. If caller of fsync(2) didn't > worry about cost, then why should we? I was thinking more from the point of view of metadata copy up patches. For a metacopy file, I was thinking if I can just issue fsync() on upper file and skip it on lower file. Anyway, I don't have any strong opinion here. Vivek
On Mon, Apr 23, 2018 at 3:53 PM, Vivek Goyal <vgoyal@redhat.com> wrote: > On Mon, Apr 23, 2018 at 03:39:45PM +0200, Miklos Szeredi wrote: >> On Mon, Apr 23, 2018 at 3:36 PM, Vivek Goyal <vgoyal@redhat.com> wrote: >> > On Thu, Apr 12, 2018 at 05:08:04PM +0200, Miklos Szeredi wrote: >> >> Implement stacked fsync(). >> >> >> >> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> >> >> --- >> >> fs/overlayfs/file.c | 20 ++++++++++++++++++++ >> >> 1 file changed, 20 insertions(+) >> >> >> >> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c >> >> index b98204c1c19c..4417527667ff 100644 >> >> --- a/fs/overlayfs/file.c >> >> +++ b/fs/overlayfs/file.c >> >> @@ -222,10 +222,30 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) >> >> return ret; >> >> } >> >> >> >> +static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) >> >> +{ >> >> + struct fd real; >> >> + const struct cred *old_cred; >> >> + int ret; >> >> + >> >> + ret = ovl_real_file(file, &real); >> >> + if (ret) >> >> + return ret; >> >> + >> >> + old_cred = ovl_override_creds(file_inode(file)->i_sb); >> >> + ret = vfs_fsync_range(real.file, start, end, datasync); >> >> + revert_creds(old_cred); >> > >> > Can we avoid calling fsync() on real file if it is not upper. Is it worth >> > optimizing. >> >> Not sure it's worth bothering with. If caller of fsync(2) didn't >> worry about cost, then why should we? > > I was thinking more from the point of view of metadata copy up patches. > For a metacopy file, I was thinking if I can just issue fsync() on upper > file and skip it on lower file. Ah, in that case I agree with doing fsync only on upper. If there's a choice in implementing the given functionality (and performance doesn't matter) then the simplest one should be chosen. Thanks, Miklos
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index b98204c1c19c..4417527667ff 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -222,10 +222,30 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) return ret; } +static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) +{ + struct fd real; + const struct cred *old_cred; + int ret; + + ret = ovl_real_file(file, &real); + if (ret) + return ret; + + old_cred = ovl_override_creds(file_inode(file)->i_sb); + ret = vfs_fsync_range(real.file, start, end, datasync); + revert_creds(old_cred); + + fdput(real); + + return ret; +} + const struct file_operations ovl_file_operations = { .open = ovl_open, .release = ovl_release, .llseek = ovl_llseek, .read_iter = ovl_read_iter, .write_iter = ovl_write_iter, + .fsync = ovl_fsync, };
Implement stacked fsync(). Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/overlayfs/file.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)