* [PATCH] btrfs: make reflinks respect O_SYNC O_DSYNC and S_SYNC flags
@ 2021-03-23 18:39 fdmanana
2021-03-29 18:46 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: fdmanana @ 2021-03-23 18:39 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
If we reflink to or from a file opened with O_SYNC/O_DSYNC or to/from a
file that has the S_SYNC attribute set, we totally ignore that and do not
durably persist the reflink changes. Since a reflink can change the data
readable from a file (and mtime/ctime, or a file size), it makes sense to
durably persist (fsync) the source and destination files/ranges.
This was previously discussed at:
https://lore.kernel.org/linux-btrfs/20200903035225.GJ6090@magnolia/
The recently introduced test case generic/628, from fstests, exercises
these scenarios and currently fails without this change.
So make sure we fsync the source and destination files/ranges when either
of them was opened with O_SYNC/O_DSYNC or has the S_SYNC attribute set,
just like XFS already does.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/reflink.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index 6746460fd219..598105574d05 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -834,6 +834,16 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
len, remap_flags);
}
+static bool file_sync_write(struct file *file)
+{
+ if (file->f_flags & (__O_SYNC | O_DSYNC))
+ return true;
+ if (IS_SYNC(file_inode(file)))
+ return true;
+
+ return false;
+}
+
loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
struct file *dst_file, loff_t destoff, loff_t len,
unsigned int remap_flags)
@@ -871,5 +881,19 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
unlock_two_nondirectories(src_inode, dst_inode);
}
+ /*
+ * If either the source or the destination file was opened with O_SYNC,
+ * O_DSYNC or has the S_SYNC attribute, fsync both the destination and
+ * source files/ranges, so that after a successful return (0) followed
+ * by a power failure results in the reflinked data to be readable from
+ * both files/ranges.
+ */
+ if (ret == 0 && (file_sync_write(src_file) || file_sync_write(dst_file))) {
+ ret = btrfs_sync_file(src_file, off, off + len - 1, 0);
+ if (ret == 0)
+ ret = btrfs_sync_file(dst_file, destoff,
+ destoff + len - 1, 0);
+ }
+
return ret < 0 ? ret : len;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: make reflinks respect O_SYNC O_DSYNC and S_SYNC flags
2021-03-23 18:39 [PATCH] btrfs: make reflinks respect O_SYNC O_DSYNC and S_SYNC flags fdmanana
@ 2021-03-29 18:46 ` David Sterba
2021-03-31 11:07 ` Filipe Manana
0 siblings, 1 reply; 4+ messages in thread
From: David Sterba @ 2021-03-29 18:46 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Tue, Mar 23, 2021 at 06:39:49PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> If we reflink to or from a file opened with O_SYNC/O_DSYNC or to/from a
> file that has the S_SYNC attribute set, we totally ignore that and do not
> durably persist the reflink changes. Since a reflink can change the data
> readable from a file (and mtime/ctime, or a file size), it makes sense to
> durably persist (fsync) the source and destination files/ranges.
>
> This was previously discussed at:
>
> https://lore.kernel.org/linux-btrfs/20200903035225.GJ6090@magnolia/
>
> The recently introduced test case generic/628, from fstests, exercises
> these scenarios and currently fails without this change.
>
> So make sure we fsync the source and destination files/ranges when either
> of them was opened with O_SYNC/O_DSYNC or has the S_SYNC attribute set,
> just like XFS already does.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: make reflinks respect O_SYNC O_DSYNC and S_SYNC flags
2021-03-29 18:46 ` David Sterba
@ 2021-03-31 11:07 ` Filipe Manana
2021-04-01 15:32 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Filipe Manana @ 2021-03-31 11:07 UTC (permalink / raw)
To: dsterba, Filipe Manana, linux-btrfs
On Mon, Mar 29, 2021 at 7:49 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Tue, Mar 23, 2021 at 06:39:49PM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > If we reflink to or from a file opened with O_SYNC/O_DSYNC or to/from a
> > file that has the S_SYNC attribute set, we totally ignore that and do not
> > durably persist the reflink changes. Since a reflink can change the data
> > readable from a file (and mtime/ctime, or a file size), it makes sense to
> > durably persist (fsync) the source and destination files/ranges.
> >
> > This was previously discussed at:
> >
> > https://lore.kernel.org/linux-btrfs/20200903035225.GJ6090@magnolia/
> >
> > The recently introduced test case generic/628, from fstests, exercises
> > these scenarios and currently fails without this change.
> >
> > So make sure we fsync the source and destination files/ranges when either
> > of them was opened with O_SYNC/O_DSYNC or has the S_SYNC attribute set,
> > just like XFS already does.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
>
> Added to misc-next, thanks.
Can you squash the following diff into it?
https://pastebin.com/raw/ARSSDDxd
Or if you prefer I send a v2, it's fine as well. Let me know, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: make reflinks respect O_SYNC O_DSYNC and S_SYNC flags
2021-03-31 11:07 ` Filipe Manana
@ 2021-04-01 15:32 ` David Sterba
0 siblings, 0 replies; 4+ messages in thread
From: David Sterba @ 2021-04-01 15:32 UTC (permalink / raw)
To: Filipe Manana; +Cc: dsterba, linux-btrfs
On Wed, Mar 31, 2021 at 11:07:26AM +0000, Filipe Manana wrote:
> On Mon, Mar 29, 2021 at 7:49 PM David Sterba <dsterba@suse.cz> wrote:
> >
> > On Tue, Mar 23, 2021 at 06:39:49PM +0000, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > If we reflink to or from a file opened with O_SYNC/O_DSYNC or to/from a
> > > file that has the S_SYNC attribute set, we totally ignore that and do not
> > > durably persist the reflink changes. Since a reflink can change the data
> > > readable from a file (and mtime/ctime, or a file size), it makes sense to
> > > durably persist (fsync) the source and destination files/ranges.
> > >
> > > This was previously discussed at:
> > >
> > > https://lore.kernel.org/linux-btrfs/20200903035225.GJ6090@magnolia/
> > >
> > > The recently introduced test case generic/628, from fstests, exercises
> > > these scenarios and currently fails without this change.
> > >
> > > So make sure we fsync the source and destination files/ranges when either
> > > of them was opened with O_SYNC/O_DSYNC or has the S_SYNC attribute set,
> > > just like XFS already does.
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> >
> > Added to misc-next, thanks.
>
> Can you squash the following diff into it?
>
> https://pastebin.com/raw/ARSSDDxd
Squashed in, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-01 18:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 18:39 [PATCH] btrfs: make reflinks respect O_SYNC O_DSYNC and S_SYNC flags fdmanana
2021-03-29 18:46 ` David Sterba
2021-03-31 11:07 ` Filipe Manana
2021-04-01 15:32 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.