* [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper
@ 2020-04-03 12:55 Christoph Hellwig
2020-04-03 12:55 ` [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Christoph Hellwig @ 2020-04-03 12:55 UTC (permalink / raw)
To: linux-xfs
Create a new helper to force the log up to the last LSN touching an
inode.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_export.c | 14 +-------------
fs/xfs/xfs_file.c | 12 +-----------
fs/xfs/xfs_inode.c | 19 +++++++++++++++++++
fs/xfs/xfs_inode.h | 1 +
4 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
index f1372f9046e3..5a4b0119143a 100644
--- a/fs/xfs/xfs_export.c
+++ b/fs/xfs/xfs_export.c
@@ -15,7 +15,6 @@
#include "xfs_trans.h"
#include "xfs_inode_item.h"
#include "xfs_icache.h"
-#include "xfs_log.h"
#include "xfs_pnfs.h"
/*
@@ -221,18 +220,7 @@ STATIC int
xfs_fs_nfs_commit_metadata(
struct inode *inode)
{
- struct xfs_inode *ip = XFS_I(inode);
- struct xfs_mount *mp = ip->i_mount;
- xfs_lsn_t lsn = 0;
-
- xfs_ilock(ip, XFS_ILOCK_SHARED);
- if (xfs_ipincount(ip))
- lsn = ip->i_itemp->ili_last_lsn;
- xfs_iunlock(ip, XFS_ILOCK_SHARED);
-
- if (!lsn)
- return 0;
- return xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL);
+ return xfs_log_force_inode(XFS_I(inode));
}
const struct export_operations xfs_export_operations = {
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index b8a4a3f29b36..68e1cbb3cfcc 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -80,19 +80,9 @@ xfs_dir_fsync(
int datasync)
{
struct xfs_inode *ip = XFS_I(file->f_mapping->host);
- struct xfs_mount *mp = ip->i_mount;
- xfs_lsn_t lsn = 0;
trace_xfs_dir_fsync(ip);
-
- xfs_ilock(ip, XFS_ILOCK_SHARED);
- if (xfs_ipincount(ip))
- lsn = ip->i_itemp->ili_last_lsn;
- xfs_iunlock(ip, XFS_ILOCK_SHARED);
-
- if (!lsn)
- return 0;
- return xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL);
+ return xfs_log_force_inode(ip);
}
STATIC int
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c5077e6326c7..e48fc835cb85 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3951,3 +3951,22 @@ xfs_irele(
trace_xfs_irele(ip, _RET_IP_);
iput(VFS_I(ip));
}
+
+/*
+ * Ensure all commited transactions touching the inode are written to the log.
+ */
+int
+xfs_log_force_inode(
+ struct xfs_inode *ip)
+{
+ xfs_lsn_t lsn = 0;
+
+ xfs_ilock(ip, XFS_ILOCK_SHARED);
+ if (xfs_ipincount(ip))
+ lsn = ip->i_itemp->ili_last_lsn;
+ xfs_iunlock(ip, XFS_ILOCK_SHARED);
+
+ if (!lsn)
+ return 0;
+ return xfs_log_force_lsn(ip->i_mount, lsn, XFS_LOG_SYNC, NULL);
+}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 492e53992fa9..c6a63f6764a6 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -426,6 +426,7 @@ int xfs_itruncate_extents_flags(struct xfs_trans **,
struct xfs_inode *, int, xfs_fsize_t, int);
void xfs_iext_realloc(xfs_inode_t *, int, int);
+int xfs_log_force_inode(struct xfs_inode *ip);
void xfs_iunpin_wait(xfs_inode_t *);
#define xfs_ipincount(ip) ((unsigned int) atomic_read(&ip->i_pincount))
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync
2020-04-03 12:55 [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper Christoph Hellwig
@ 2020-04-03 12:55 ` Christoph Hellwig
2020-04-03 15:42 ` Darrick J. Wong
2020-04-06 12:14 ` Brian Foster
2020-04-03 15:42 ` [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper Darrick J. Wong
2020-04-06 12:13 ` Brian Foster
2 siblings, 2 replies; 9+ messages in thread
From: Christoph Hellwig @ 2020-04-03 12:55 UTC (permalink / raw)
To: linux-xfs
Reflink should force the log out to disk if the filesystem was mounted
with wsync, the same as most other operations in xfs.
Fixes: 3fc9f5e409319 ("xfs: remove xfs_reflink_remap_range")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_file.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 68e1cbb3cfcc..4b8bdecc3863 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1059,7 +1059,11 @@ xfs_file_remap_range(
ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize,
remap_flags);
+ if (ret)
+ goto out_unlock;
+ if (mp->m_flags & XFS_MOUNT_WSYNC)
+ xfs_log_force_inode(dest);
out_unlock:
xfs_reflink_remap_unlock(file_in, file_out);
if (ret)
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper
2020-04-03 12:55 [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper Christoph Hellwig
2020-04-03 12:55 ` [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync Christoph Hellwig
@ 2020-04-03 15:42 ` Darrick J. Wong
2020-04-06 12:13 ` Brian Foster
2 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2020-04-03 15:42 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Apr 03, 2020 at 02:55:21PM +0200, Christoph Hellwig wrote:
> Create a new helper to force the log up to the last LSN touching an
> inode.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks reasonable,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_export.c | 14 +-------------
> fs/xfs/xfs_file.c | 12 +-----------
> fs/xfs/xfs_inode.c | 19 +++++++++++++++++++
> fs/xfs/xfs_inode.h | 1 +
> 4 files changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
> index f1372f9046e3..5a4b0119143a 100644
> --- a/fs/xfs/xfs_export.c
> +++ b/fs/xfs/xfs_export.c
> @@ -15,7 +15,6 @@
> #include "xfs_trans.h"
> #include "xfs_inode_item.h"
> #include "xfs_icache.h"
> -#include "xfs_log.h"
> #include "xfs_pnfs.h"
>
> /*
> @@ -221,18 +220,7 @@ STATIC int
> xfs_fs_nfs_commit_metadata(
> struct inode *inode)
> {
> - struct xfs_inode *ip = XFS_I(inode);
> - struct xfs_mount *mp = ip->i_mount;
> - xfs_lsn_t lsn = 0;
> -
> - xfs_ilock(ip, XFS_ILOCK_SHARED);
> - if (xfs_ipincount(ip))
> - lsn = ip->i_itemp->ili_last_lsn;
> - xfs_iunlock(ip, XFS_ILOCK_SHARED);
> -
> - if (!lsn)
> - return 0;
> - return xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL);
> + return xfs_log_force_inode(XFS_I(inode));
> }
>
> const struct export_operations xfs_export_operations = {
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index b8a4a3f29b36..68e1cbb3cfcc 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -80,19 +80,9 @@ xfs_dir_fsync(
> int datasync)
> {
> struct xfs_inode *ip = XFS_I(file->f_mapping->host);
> - struct xfs_mount *mp = ip->i_mount;
> - xfs_lsn_t lsn = 0;
>
> trace_xfs_dir_fsync(ip);
> -
> - xfs_ilock(ip, XFS_ILOCK_SHARED);
> - if (xfs_ipincount(ip))
> - lsn = ip->i_itemp->ili_last_lsn;
> - xfs_iunlock(ip, XFS_ILOCK_SHARED);
> -
> - if (!lsn)
> - return 0;
> - return xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL);
> + return xfs_log_force_inode(ip);
> }
>
> STATIC int
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..e48fc835cb85 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3951,3 +3951,22 @@ xfs_irele(
> trace_xfs_irele(ip, _RET_IP_);
> iput(VFS_I(ip));
> }
> +
> +/*
> + * Ensure all commited transactions touching the inode are written to the log.
> + */
> +int
> +xfs_log_force_inode(
> + struct xfs_inode *ip)
> +{
> + xfs_lsn_t lsn = 0;
> +
> + xfs_ilock(ip, XFS_ILOCK_SHARED);
> + if (xfs_ipincount(ip))
> + lsn = ip->i_itemp->ili_last_lsn;
> + xfs_iunlock(ip, XFS_ILOCK_SHARED);
> +
> + if (!lsn)
> + return 0;
> + return xfs_log_force_lsn(ip->i_mount, lsn, XFS_LOG_SYNC, NULL);
> +}
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 492e53992fa9..c6a63f6764a6 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -426,6 +426,7 @@ int xfs_itruncate_extents_flags(struct xfs_trans **,
> struct xfs_inode *, int, xfs_fsize_t, int);
> void xfs_iext_realloc(xfs_inode_t *, int, int);
>
> +int xfs_log_force_inode(struct xfs_inode *ip);
> void xfs_iunpin_wait(xfs_inode_t *);
> #define xfs_ipincount(ip) ((unsigned int) atomic_read(&ip->i_pincount))
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync
2020-04-03 12:55 ` [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync Christoph Hellwig
@ 2020-04-03 15:42 ` Darrick J. Wong
2020-04-06 12:14 ` Brian Foster
1 sibling, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2020-04-03 15:42 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Apr 03, 2020 at 02:55:22PM +0200, Christoph Hellwig wrote:
> Reflink should force the log out to disk if the filesystem was mounted
> with wsync, the same as most other operations in xfs.
>
> Fixes: 3fc9f5e409319 ("xfs: remove xfs_reflink_remap_range")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Seems like a cleaner implementation than the one I came up with,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_file.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 68e1cbb3cfcc..4b8bdecc3863 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1059,7 +1059,11 @@ xfs_file_remap_range(
>
> ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize,
> remap_flags);
> + if (ret)
> + goto out_unlock;
>
> + if (mp->m_flags & XFS_MOUNT_WSYNC)
> + xfs_log_force_inode(dest);
> out_unlock:
> xfs_reflink_remap_unlock(file_in, file_out);
> if (ret)
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper
2020-04-03 12:55 [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper Christoph Hellwig
2020-04-03 12:55 ` [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync Christoph Hellwig
2020-04-03 15:42 ` [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper Darrick J. Wong
@ 2020-04-06 12:13 ` Brian Foster
2 siblings, 0 replies; 9+ messages in thread
From: Brian Foster @ 2020-04-06 12:13 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Apr 03, 2020 at 02:55:21PM +0200, Christoph Hellwig wrote:
> Create a new helper to force the log up to the last LSN touching an
> inode.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_export.c | 14 +-------------
> fs/xfs/xfs_file.c | 12 +-----------
> fs/xfs/xfs_inode.c | 19 +++++++++++++++++++
> fs/xfs/xfs_inode.h | 1 +
> 4 files changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
> index f1372f9046e3..5a4b0119143a 100644
> --- a/fs/xfs/xfs_export.c
> +++ b/fs/xfs/xfs_export.c
> @@ -15,7 +15,6 @@
> #include "xfs_trans.h"
> #include "xfs_inode_item.h"
> #include "xfs_icache.h"
> -#include "xfs_log.h"
> #include "xfs_pnfs.h"
>
> /*
> @@ -221,18 +220,7 @@ STATIC int
> xfs_fs_nfs_commit_metadata(
> struct inode *inode)
> {
> - struct xfs_inode *ip = XFS_I(inode);
> - struct xfs_mount *mp = ip->i_mount;
> - xfs_lsn_t lsn = 0;
> -
> - xfs_ilock(ip, XFS_ILOCK_SHARED);
> - if (xfs_ipincount(ip))
> - lsn = ip->i_itemp->ili_last_lsn;
> - xfs_iunlock(ip, XFS_ILOCK_SHARED);
> -
> - if (!lsn)
> - return 0;
> - return xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL);
> + return xfs_log_force_inode(XFS_I(inode));
> }
>
> const struct export_operations xfs_export_operations = {
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index b8a4a3f29b36..68e1cbb3cfcc 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -80,19 +80,9 @@ xfs_dir_fsync(
> int datasync)
> {
> struct xfs_inode *ip = XFS_I(file->f_mapping->host);
> - struct xfs_mount *mp = ip->i_mount;
> - xfs_lsn_t lsn = 0;
>
> trace_xfs_dir_fsync(ip);
> -
> - xfs_ilock(ip, XFS_ILOCK_SHARED);
> - if (xfs_ipincount(ip))
> - lsn = ip->i_itemp->ili_last_lsn;
> - xfs_iunlock(ip, XFS_ILOCK_SHARED);
> -
> - if (!lsn)
> - return 0;
> - return xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL);
> + return xfs_log_force_inode(ip);
> }
>
> STATIC int
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..e48fc835cb85 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3951,3 +3951,22 @@ xfs_irele(
> trace_xfs_irele(ip, _RET_IP_);
> iput(VFS_I(ip));
> }
> +
> +/*
> + * Ensure all commited transactions touching the inode are written to the log.
> + */
> +int
> +xfs_log_force_inode(
> + struct xfs_inode *ip)
> +{
> + xfs_lsn_t lsn = 0;
> +
> + xfs_ilock(ip, XFS_ILOCK_SHARED);
> + if (xfs_ipincount(ip))
> + lsn = ip->i_itemp->ili_last_lsn;
> + xfs_iunlock(ip, XFS_ILOCK_SHARED);
> +
> + if (!lsn)
> + return 0;
> + return xfs_log_force_lsn(ip->i_mount, lsn, XFS_LOG_SYNC, NULL);
> +}
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 492e53992fa9..c6a63f6764a6 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -426,6 +426,7 @@ int xfs_itruncate_extents_flags(struct xfs_trans **,
> struct xfs_inode *, int, xfs_fsize_t, int);
> void xfs_iext_realloc(xfs_inode_t *, int, int);
>
> +int xfs_log_force_inode(struct xfs_inode *ip);
> void xfs_iunpin_wait(xfs_inode_t *);
> #define xfs_ipincount(ip) ((unsigned int) atomic_read(&ip->i_pincount))
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync
2020-04-03 12:55 ` [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync Christoph Hellwig
2020-04-03 15:42 ` Darrick J. Wong
@ 2020-04-06 12:14 ` Brian Foster
2020-04-06 15:31 ` Darrick J. Wong
1 sibling, 1 reply; 9+ messages in thread
From: Brian Foster @ 2020-04-06 12:14 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Apr 03, 2020 at 02:55:22PM +0200, Christoph Hellwig wrote:
> Reflink should force the log out to disk if the filesystem was mounted
> with wsync, the same as most other operations in xfs.
>
Isn't WSYNC for namespace operations? Why is this needed for reflink?
> Fixes: 3fc9f5e409319 ("xfs: remove xfs_reflink_remap_range")
At a glance this looks like a refactoring patch. What does this fix?
Brian
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_file.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 68e1cbb3cfcc..4b8bdecc3863 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1059,7 +1059,11 @@ xfs_file_remap_range(
>
> ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize,
> remap_flags);
> + if (ret)
> + goto out_unlock;
>
> + if (mp->m_flags & XFS_MOUNT_WSYNC)
> + xfs_log_force_inode(dest);
> out_unlock:
> xfs_reflink_remap_unlock(file_in, file_out);
> if (ret)
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync
2020-04-06 12:14 ` Brian Foster
@ 2020-04-06 15:31 ` Darrick J. Wong
2020-04-06 16:04 ` Brian Foster
0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2020-04-06 15:31 UTC (permalink / raw)
To: Brian Foster; +Cc: Christoph Hellwig, linux-xfs
On Mon, Apr 06, 2020 at 08:14:37AM -0400, Brian Foster wrote:
> On Fri, Apr 03, 2020 at 02:55:22PM +0200, Christoph Hellwig wrote:
> > Reflink should force the log out to disk if the filesystem was mounted
> > with wsync, the same as most other operations in xfs.
> >
>
> Isn't WSYNC for namespace operations? Why is this needed for reflink?
The manpage says that 'wsync' (the mount option) is for making namespace
operations synchronous.
However, xfs_init_fs_context sets XFS_MOUNT_WSYNC if the admin set
the 'sync' mount option, which makes all IO synchronous.
> > Fixes: 3fc9f5e409319 ("xfs: remove xfs_reflink_remap_range")
>
> At a glance this looks like a refactoring patch. What does this fix?
It probably ought to be 862bb360ef569f ("xfs: reflink extents from one
file to another") but so much of that was refactored for 5.0 that
backporting this fix will require changing a totally different function
(xfs_reflink_remap_range) in a totally different file (xfs_reflink.c).
--D
> Brian
>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> > fs/xfs/xfs_file.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index 68e1cbb3cfcc..4b8bdecc3863 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -1059,7 +1059,11 @@ xfs_file_remap_range(
> >
> > ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize,
> > remap_flags);
> > + if (ret)
> > + goto out_unlock;
> >
> > + if (mp->m_flags & XFS_MOUNT_WSYNC)
> > + xfs_log_force_inode(dest);
> > out_unlock:
> > xfs_reflink_remap_unlock(file_in, file_out);
> > if (ret)
> > --
> > 2.25.1
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync
2020-04-06 15:31 ` Darrick J. Wong
@ 2020-04-06 16:04 ` Brian Foster
2020-04-06 16:22 ` Darrick J. Wong
0 siblings, 1 reply; 9+ messages in thread
From: Brian Foster @ 2020-04-06 16:04 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Mon, Apr 06, 2020 at 08:31:54AM -0700, Darrick J. Wong wrote:
> On Mon, Apr 06, 2020 at 08:14:37AM -0400, Brian Foster wrote:
> > On Fri, Apr 03, 2020 at 02:55:22PM +0200, Christoph Hellwig wrote:
> > > Reflink should force the log out to disk if the filesystem was mounted
> > > with wsync, the same as most other operations in xfs.
> > >
> >
> > Isn't WSYNC for namespace operations? Why is this needed for reflink?
>
> The manpage says that 'wsync' (the mount option) is for making namespace
> operations synchronous.
>
> However, xfs_init_fs_context sets XFS_MOUNT_WSYNC if the admin set
> the 'sync' mount option, which makes all IO synchronous.
>
Ok.. so we're considering reflink a form of I/O.. I suppose that makes
sense, though it would be nice to explain that in the commit log...
Reviewed-by: Brian Foster <bfoster@redhat.com>
> > > Fixes: 3fc9f5e409319 ("xfs: remove xfs_reflink_remap_range")
> >
> > At a glance this looks like a refactoring patch. What does this fix?
>
> It probably ought to be 862bb360ef569f ("xfs: reflink extents from one
> file to another") but so much of that was refactored for 5.0 that
> backporting this fix will require changing a totally different function
> (xfs_reflink_remap_range) in a totally different file (xfs_reflink.c).
>
> --D
>
> > Brian
> >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > > fs/xfs/xfs_file.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > index 68e1cbb3cfcc..4b8bdecc3863 100644
> > > --- a/fs/xfs/xfs_file.c
> > > +++ b/fs/xfs/xfs_file.c
> > > @@ -1059,7 +1059,11 @@ xfs_file_remap_range(
> > >
> > > ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize,
> > > remap_flags);
> > > + if (ret)
> > > + goto out_unlock;
> > >
> > > + if (mp->m_flags & XFS_MOUNT_WSYNC)
> > > + xfs_log_force_inode(dest);
> > > out_unlock:
> > > xfs_reflink_remap_unlock(file_in, file_out);
> > > if (ret)
> > > --
> > > 2.25.1
> > >
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync
2020-04-06 16:04 ` Brian Foster
@ 2020-04-06 16:22 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2020-04-06 16:22 UTC (permalink / raw)
To: Brian Foster; +Cc: Christoph Hellwig, linux-xfs
On Mon, Apr 06, 2020 at 12:04:37PM -0400, Brian Foster wrote:
> On Mon, Apr 06, 2020 at 08:31:54AM -0700, Darrick J. Wong wrote:
> > On Mon, Apr 06, 2020 at 08:14:37AM -0400, Brian Foster wrote:
> > > On Fri, Apr 03, 2020 at 02:55:22PM +0200, Christoph Hellwig wrote:
> > > > Reflink should force the log out to disk if the filesystem was mounted
> > > > with wsync, the same as most other operations in xfs.
> > > >
> > >
> > > Isn't WSYNC for namespace operations? Why is this needed for reflink?
> >
> > The manpage says that 'wsync' (the mount option) is for making namespace
> > operations synchronous.
> >
> > However, xfs_init_fs_context sets XFS_MOUNT_WSYNC if the admin set
> > the 'sync' mount option, which makes all IO synchronous.
> >
>
> Ok.. so we're considering reflink a form of I/O.. I suppose that makes
> sense, though it would be nice to explain that in the commit log...
Ok, I'll add the following:
[Note: XFS_MOUNT_WSYNC is set when the admin mounts the filesystem
with either the 'wsync' or 'sync' mount options, which effectively means
that we're classifying reflink/dedupe as IO operations and making them
synchronous when required.]
Thanks for reviewing. :)
--D
> Reviewed-by: Brian Foster <bfoster@redhat.com>
>
> > > > Fixes: 3fc9f5e409319 ("xfs: remove xfs_reflink_remap_range")
> > >
> > > At a glance this looks like a refactoring patch. What does this fix?
> >
> > It probably ought to be 862bb360ef569f ("xfs: reflink extents from one
> > file to another") but so much of that was refactored for 5.0 that
> > backporting this fix will require changing a totally different function
> > (xfs_reflink_remap_range) in a totally different file (xfs_reflink.c).
> >
> > --D
> >
> > > Brian
> > >
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > ---
> > > > fs/xfs/xfs_file.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > > index 68e1cbb3cfcc..4b8bdecc3863 100644
> > > > --- a/fs/xfs/xfs_file.c
> > > > +++ b/fs/xfs/xfs_file.c
> > > > @@ -1059,7 +1059,11 @@ xfs_file_remap_range(
> > > >
> > > > ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize,
> > > > remap_flags);
> > > > + if (ret)
> > > > + goto out_unlock;
> > > >
> > > > + if (mp->m_flags & XFS_MOUNT_WSYNC)
> > > > + xfs_log_force_inode(dest);
> > > > out_unlock:
> > > > xfs_reflink_remap_unlock(file_in, file_out);
> > > > if (ret)
> > > > --
> > > > 2.25.1
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-04-06 16:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 12:55 [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper Christoph Hellwig
2020-04-03 12:55 ` [PATCH 2/2] xfs: reflink should force the log out if mounted with wsync Christoph Hellwig
2020-04-03 15:42 ` Darrick J. Wong
2020-04-06 12:14 ` Brian Foster
2020-04-06 15:31 ` Darrick J. Wong
2020-04-06 16:04 ` Brian Foster
2020-04-06 16:22 ` Darrick J. Wong
2020-04-03 15:42 ` [PATCH 1/2] xfs: factor out a new xfs_log_force_inode helper Darrick J. Wong
2020-04-06 12:13 ` Brian Foster
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.