* [PATCH 1/5] xfs: don't log every time we clear the log incompat flags
2022-05-24 5:36 [PATCHSET v2 0/5] xfs: last pile of LARP cleanups for 5.19 Darrick J. Wong
@ 2022-05-24 5:36 ` Darrick J. Wong
2022-05-27 1:26 ` Dave Chinner
2022-05-24 5:36 ` [PATCH 2/5] xfs: implement per-mount warnings for scrub and shrink usage Darrick J. Wong
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-05-24 5:36 UTC (permalink / raw)
To: djwong; +Cc: linux-xfs, david, allison.henderson
From: Darrick J. Wong <djwong@kernel.org>
There's no need to spam the logs every time we clear the log incompat
flags -- if someone is periodically using one of these features, they'll
be cleared every time the log tries to clean itself, which can get
pretty chatty:
$ dmesg | grep -i clear
[ 5363.894711] XFS (sdd): Clearing log incompat feature flags.
[ 5365.157516] XFS (sdd): Clearing log incompat feature flags.
[ 5369.388543] XFS (sdd): Clearing log incompat feature flags.
[ 5371.281246] XFS (sdd): Clearing log incompat feature flags.
These aren't high value messages either -- nothing's gone wrong, and
nobody's trying anything tricky.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/xfs_mount.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 0c0bcbd4949d..daa8d29c46b4 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1356,7 +1356,6 @@ xfs_clear_incompat_log_features(
if (xfs_sb_has_incompat_log_feature(&mp->m_sb,
XFS_SB_FEAT_INCOMPAT_LOG_ALL)) {
- xfs_info(mp, "Clearing log incompat feature flags.");
xfs_sb_remove_incompat_log_features(&mp->m_sb);
ret = true;
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/5] xfs: don't log every time we clear the log incompat flags
2022-05-24 5:36 ` [PATCH 1/5] xfs: don't log every time we clear the log incompat flags Darrick J. Wong
@ 2022-05-27 1:26 ` Dave Chinner
0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2022-05-27 1:26 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, allison.henderson
On Mon, May 23, 2022 at 10:36:25PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> There's no need to spam the logs every time we clear the log incompat
> flags -- if someone is periodically using one of these features, they'll
> be cleared every time the log tries to clean itself, which can get
> pretty chatty:
>
> $ dmesg | grep -i clear
> [ 5363.894711] XFS (sdd): Clearing log incompat feature flags.
> [ 5365.157516] XFS (sdd): Clearing log incompat feature flags.
> [ 5369.388543] XFS (sdd): Clearing log incompat feature flags.
> [ 5371.281246] XFS (sdd): Clearing log incompat feature flags.
>
> These aren't high value messages either -- nothing's gone wrong, and
> nobody's trying anything tricky.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Make sense.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/5] xfs: implement per-mount warnings for scrub and shrink usage
2022-05-24 5:36 [PATCHSET v2 0/5] xfs: last pile of LARP cleanups for 5.19 Darrick J. Wong
2022-05-24 5:36 ` [PATCH 1/5] xfs: don't log every time we clear the log incompat flags Darrick J. Wong
@ 2022-05-24 5:36 ` Darrick J. Wong
2022-05-27 1:26 ` Dave Chinner
2022-05-24 5:36 ` [PATCH 3/5] xfs: warn about LARP once per mount Darrick J. Wong
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-05-24 5:36 UTC (permalink / raw)
To: djwong; +Cc: linux-xfs, david, allison.henderson
From: Darrick J. Wong <djwong@kernel.org>
Currently, we don't have a consistent story around logging when an
EXPERIMENTAL feature gets turned on at runtime -- online fsck and shrink
log a message once per day across all mounts, and the recently merged
LARP mode only ever does it once per insmod cycle or reboot.
Because EXPERIMENTAL tags are supposed to go away eventually, convert
the existing daily warnings into state flags that travel with the mount,
and warn once per mount. Making this an opstate flag means that we'll
be able to capture the experimental usage in the ftrace output too.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/scrub/scrub.c | 17 ++---------------
fs/xfs/xfs_fsops.c | 7 +------
fs/xfs/xfs_message.h | 6 ++++++
fs/xfs/xfs_mount.h | 15 ++++++++++++++-
4 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index b11870d07c56..2e8e400f10a9 100644
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -340,20 +340,6 @@ static const struct xchk_meta_ops meta_scrub_ops[] = {
},
};
-/* This isn't a stable feature, warn once per day. */
-static inline void
-xchk_experimental_warning(
- struct xfs_mount *mp)
-{
- static struct ratelimit_state scrub_warning = RATELIMIT_STATE_INIT(
- "xchk_warning", 86400 * HZ, 1);
- ratelimit_set_flags(&scrub_warning, RATELIMIT_MSG_ON_RELEASE);
-
- if (__ratelimit(&scrub_warning))
- xfs_alert(mp,
-"EXPERIMENTAL online scrub feature in use. Use at your own risk!");
-}
-
static int
xchk_validate_inputs(
struct xfs_mount *mp,
@@ -478,7 +464,8 @@ xfs_scrub_metadata(
if (error)
goto out;
- xchk_experimental_warning(mp);
+ xfs_warn_mount(mp, XFS_OPSTATE_WARNED_SCRUB,
+ "EXPERIMENTAL online scrub feature in use. Use at your own risk!");
sc = kmem_zalloc(sizeof(struct xfs_scrub), KM_NOFS | KM_MAYFAIL);
if (!sc) {
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 888839e75d11..d4a77c53f94b 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -149,12 +149,7 @@ xfs_growfs_data_private(
error = xfs_resizefs_init_new_ags(tp, &id, oagcount, nagcount,
delta, &lastag_extended);
} else {
- static struct ratelimit_state shrink_warning = \
- RATELIMIT_STATE_INIT("shrink_warning", 86400 * HZ, 1);
- ratelimit_set_flags(&shrink_warning, RATELIMIT_MSG_ON_RELEASE);
-
- if (__ratelimit(&shrink_warning))
- xfs_alert(mp,
+ xfs_warn_mount(mp, XFS_OPSTATE_WARNED_SHRINK,
"EXPERIMENTAL online shrink feature in use. Use at your own risk!");
error = xfs_ag_shrink_space(mp, &tp, nagcount - 1, -delta);
diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
index 55ee464ab59f..cc323775a12c 100644
--- a/fs/xfs/xfs_message.h
+++ b/fs/xfs/xfs_message.h
@@ -75,6 +75,12 @@ do { \
#define xfs_debug_ratelimited(dev, fmt, ...) \
xfs_printk_ratelimited(xfs_debug, dev, fmt, ##__VA_ARGS__)
+#define xfs_warn_mount(mp, warntag, fmt, ...) \
+do { \
+ if (xfs_should_warn((mp), (warntag))) \
+ xfs_warn((mp), (fmt), ##__VA_ARGS__); \
+} while (0)
+
#define xfs_warn_once(dev, fmt, ...) \
xfs_printk_once(xfs_warn, dev, fmt, ##__VA_ARGS__)
#define xfs_notice_once(dev, fmt, ...) \
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 8c42786e4942..93a954271db2 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -391,6 +391,11 @@ __XFS_HAS_FEAT(nouuid, NOUUID)
*/
#define XFS_OPSTATE_BLOCKGC_ENABLED 6
+/* Kernel has logged a warning about online fsck being used on this fs. */
+#define XFS_OPSTATE_WARNED_SCRUB 7
+/* Kernel has logged a warning about shrink being used on this fs. */
+#define XFS_OPSTATE_WARNED_SHRINK 8
+
#define __XFS_IS_OPSTATE(name, NAME) \
static inline bool xfs_is_ ## name (struct xfs_mount *mp) \
{ \
@@ -413,6 +418,12 @@ __XFS_IS_OPSTATE(readonly, READONLY)
__XFS_IS_OPSTATE(inodegc_enabled, INODEGC_ENABLED)
__XFS_IS_OPSTATE(blockgc_enabled, BLOCKGC_ENABLED)
+static inline bool
+xfs_should_warn(struct xfs_mount *mp, long nr)
+{
+ return !test_and_set_bit(nr, &mp->m_opstate);
+}
+
#define XFS_OPSTATE_STRINGS \
{ (1UL << XFS_OPSTATE_UNMOUNTING), "unmounting" }, \
{ (1UL << XFS_OPSTATE_CLEAN), "clean" }, \
@@ -420,7 +431,9 @@ __XFS_IS_OPSTATE(blockgc_enabled, BLOCKGC_ENABLED)
{ (1UL << XFS_OPSTATE_INODE32), "inode32" }, \
{ (1UL << XFS_OPSTATE_READONLY), "read_only" }, \
{ (1UL << XFS_OPSTATE_INODEGC_ENABLED), "inodegc" }, \
- { (1UL << XFS_OPSTATE_BLOCKGC_ENABLED), "blockgc" }
+ { (1UL << XFS_OPSTATE_BLOCKGC_ENABLED), "blockgc" }, \
+ { (1UL << XFS_OPSTATE_WARNED_SCRUB), "wscrub" }, \
+ { (1UL << XFS_OPSTATE_WARNED_SHRINK), "wshrink" }
/*
* Max and min values for mount-option defined I/O
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] xfs: implement per-mount warnings for scrub and shrink usage
2022-05-24 5:36 ` [PATCH 2/5] xfs: implement per-mount warnings for scrub and shrink usage Darrick J. Wong
@ 2022-05-27 1:26 ` Dave Chinner
0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2022-05-27 1:26 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, allison.henderson
On Mon, May 23, 2022 at 10:36:31PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Currently, we don't have a consistent story around logging when an
> EXPERIMENTAL feature gets turned on at runtime -- online fsck and shrink
> log a message once per day across all mounts, and the recently merged
> LARP mode only ever does it once per insmod cycle or reboot.
>
> Because EXPERIMENTAL tags are supposed to go away eventually, convert
> the existing daily warnings into state flags that travel with the mount,
> and warn once per mount. Making this an opstate flag means that we'll
> be able to capture the experimental usage in the ftrace output too.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/scrub/scrub.c | 17 ++---------------
> fs/xfs/xfs_fsops.c | 7 +------
> fs/xfs/xfs_message.h | 6 ++++++
> fs/xfs/xfs_mount.h | 15 ++++++++++++++-
> 4 files changed, 23 insertions(+), 22 deletions(-)
nice - I like the xfs_warn_mount() wrapper.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] xfs: warn about LARP once per mount
2022-05-24 5:36 [PATCHSET v2 0/5] xfs: last pile of LARP cleanups for 5.19 Darrick J. Wong
2022-05-24 5:36 ` [PATCH 1/5] xfs: don't log every time we clear the log incompat flags Darrick J. Wong
2022-05-24 5:36 ` [PATCH 2/5] xfs: implement per-mount warnings for scrub and shrink usage Darrick J. Wong
@ 2022-05-24 5:36 ` Darrick J. Wong
2022-05-27 1:27 ` Dave Chinner
2022-05-24 5:36 ` [PATCH 4/5] xfs: move xfs_attr_use_log_assist out of xfs_log.c Darrick J. Wong
2022-05-24 5:36 ` [PATCH 5/5] xfs: move xfs_attr_use_log_assist usage out of libxfs Darrick J. Wong
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-05-24 5:36 UTC (permalink / raw)
To: djwong; +Cc: linux-xfs, david, allison.henderson
From: Darrick J. Wong <djwong@kernel.org>
Since LARP is an experimental debug-only feature, we should try to warn
about it being in use once per mount, not once per reboot.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/xfs_log.c | 4 ++--
fs/xfs/xfs_mount.h | 5 ++++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 9dc748abdf33..a75f4ffc75f9 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3910,8 +3910,8 @@ xfs_attr_use_log_assist(
if (error)
goto drop_incompat;
- xfs_warn_once(mp,
-"EXPERIMENTAL logged extended attributes feature added. Use at your own risk!");
+ xfs_warn_mount(mp, XFS_OPSTATE_WARNED_LARP,
+ "EXPERIMENTAL logged extended attributes feature in use. Use at your own risk!");
return 0;
drop_incompat:
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 93a954271db2..ba5d42abf66e 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -395,6 +395,8 @@ __XFS_HAS_FEAT(nouuid, NOUUID)
#define XFS_OPSTATE_WARNED_SCRUB 7
/* Kernel has logged a warning about shrink being used on this fs. */
#define XFS_OPSTATE_WARNED_SHRINK 8
+/* Kernel has logged a warning about logged xattr updates being used. */
+#define XFS_OPSTATE_WARNED_LARP 9
#define __XFS_IS_OPSTATE(name, NAME) \
static inline bool xfs_is_ ## name (struct xfs_mount *mp) \
@@ -433,7 +435,8 @@ xfs_should_warn(struct xfs_mount *mp, long nr)
{ (1UL << XFS_OPSTATE_INODEGC_ENABLED), "inodegc" }, \
{ (1UL << XFS_OPSTATE_BLOCKGC_ENABLED), "blockgc" }, \
{ (1UL << XFS_OPSTATE_WARNED_SCRUB), "wscrub" }, \
- { (1UL << XFS_OPSTATE_WARNED_SHRINK), "wshrink" }
+ { (1UL << XFS_OPSTATE_WARNED_SHRINK), "wshrink" }, \
+ { (1UL << XFS_OPSTATE_WARNED_LARP), "wlarp" }
/*
* Max and min values for mount-option defined I/O
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/5] xfs: warn about LARP once per mount
2022-05-24 5:36 ` [PATCH 3/5] xfs: warn about LARP once per mount Darrick J. Wong
@ 2022-05-27 1:27 ` Dave Chinner
0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2022-05-27 1:27 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, allison.henderson
On Mon, May 23, 2022 at 10:36:37PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Since LARP is an experimental debug-only feature, we should try to warn
> about it being in use once per mount, not once per reboot.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/xfs_log.c | 4 ++--
> fs/xfs/xfs_mount.h | 5 ++++-
> 2 files changed, 6 insertions(+), 3 deletions(-)
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] xfs: move xfs_attr_use_log_assist out of xfs_log.c
2022-05-24 5:36 [PATCHSET v2 0/5] xfs: last pile of LARP cleanups for 5.19 Darrick J. Wong
` (2 preceding siblings ...)
2022-05-24 5:36 ` [PATCH 3/5] xfs: warn about LARP once per mount Darrick J. Wong
@ 2022-05-24 5:36 ` Darrick J. Wong
2022-05-27 1:28 ` Dave Chinner
2022-05-24 5:36 ` [PATCH 5/5] xfs: move xfs_attr_use_log_assist usage out of libxfs Darrick J. Wong
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-05-24 5:36 UTC (permalink / raw)
To: djwong; +Cc: linux-xfs, david, allison.henderson
From: Darrick J. Wong <djwong@kernel.org>
The LARP patchset added an awkward coupling point between libxfs and
what would be libxlog, if the XFS log were actually its own library.
Move the code that enables logged xattr updates out of "lib"xlog and into
xfs_xattr.c so that it no longer has to know about xlog_* functions.
While we're at it, give xfs_xattr.c its own header file.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr.c | 6 +++---
fs/xfs/xfs_log.c | 41 --------------------------------------
fs/xfs/xfs_super.c | 1 +
fs/xfs/xfs_super.h | 1 -
fs/xfs/xfs_xattr.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_xattr.h | 14 +++++++++++++
6 files changed, 67 insertions(+), 45 deletions(-)
create mode 100644 fs/xfs/xfs_xattr.h
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 9f14aca29ec4..24fa213715c1 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -25,7 +25,7 @@
#include "xfs_trans_space.h"
#include "xfs_trace.h"
#include "xfs_attr_item.h"
-#include "xfs_log.h"
+#include "xfs_xattr.h"
struct kmem_cache *xfs_attr_intent_cache;
@@ -1028,7 +1028,7 @@ xfs_attr_set(
}
if (use_logging) {
- error = xfs_attr_use_log_assist(mp);
+ error = xfs_attr_grab_log_assist(mp);
if (error)
return error;
}
@@ -1102,7 +1102,7 @@ xfs_attr_set(
xfs_iunlock(dp, XFS_ILOCK_EXCL);
drop_incompat:
if (use_logging)
- xlog_drop_incompat_feat(mp->m_log);
+ xfs_attr_rele_log_assist(mp);
return error;
out_trans_cancel:
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index a75f4ffc75f9..1e972f884a81 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3877,44 +3877,3 @@ xlog_drop_incompat_feat(
{
up_read(&log->l_incompat_users);
}
-
-/*
- * Get permission to use log-assisted atomic exchange of file extents.
- *
- * Callers must not be running any transactions or hold any inode locks, and
- * they must release the permission by calling xlog_drop_incompat_feat
- * when they're done.
- */
-int
-xfs_attr_use_log_assist(
- struct xfs_mount *mp)
-{
- int error = 0;
-
- /*
- * Protect ourselves from an idle log clearing the logged xattrs log
- * incompat feature bit.
- */
- xlog_use_incompat_feat(mp->m_log);
-
- /*
- * If log-assisted xattrs are already enabled, the caller can use the
- * log assisted swap functions with the log-incompat reference we got.
- */
- if (xfs_sb_version_haslogxattrs(&mp->m_sb))
- return 0;
-
- /* Enable log-assisted xattrs. */
- error = xfs_add_incompat_log_feature(mp,
- XFS_SB_FEAT_INCOMPAT_LOG_XATTRS);
- if (error)
- goto drop_incompat;
-
- xfs_warn_mount(mp, XFS_OPSTATE_WARNED_LARP,
- "EXPERIMENTAL logged extended attributes feature in use. Use at your own risk!");
-
- return 0;
-drop_incompat:
- xlog_drop_incompat_feat(mp->m_log);
- return error;
-}
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 51ce127a0cc6..a6e7b4176faf 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -39,6 +39,7 @@
#include "xfs_ag.h"
#include "xfs_defer.h"
#include "xfs_attr_item.h"
+#include "xfs_xattr.h"
#include <linux/magic.h>
#include <linux/fs_context.h>
diff --git a/fs/xfs/xfs_super.h b/fs/xfs/xfs_super.h
index 167d23f92ffe..3cd5a51bace1 100644
--- a/fs/xfs/xfs_super.h
+++ b/fs/xfs/xfs_super.h
@@ -91,7 +91,6 @@ extern xfs_agnumber_t xfs_set_inode_alloc(struct xfs_mount *,
xfs_agnumber_t agcount);
extern const struct export_operations xfs_export_operations;
-extern const struct xattr_handler *xfs_xattr_handlers[];
extern const struct quotactl_ops xfs_quotactl_operations;
extern void xfs_reinit_percpu_counters(struct xfs_mount *mp);
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 7a044afd4c46..fc6acf7021a7 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -15,9 +15,58 @@
#include "xfs_da_btree.h"
#include "xfs_attr.h"
#include "xfs_acl.h"
+#include "xfs_log.h"
+#include "xfs_xattr.h"
#include <linux/posix_acl_xattr.h>
+/*
+ * Get permission to use log-assisted atomic exchange of file extents.
+ *
+ * Callers must not be running any transactions or hold any inode locks, and
+ * they must release the permission by calling xlog_drop_incompat_feat
+ * when they're done.
+ */
+int
+xfs_attr_grab_log_assist(
+ struct xfs_mount *mp)
+{
+ int error = 0;
+
+ /*
+ * Protect ourselves from an idle log clearing the logged xattrs log
+ * incompat feature bit.
+ */
+ xlog_use_incompat_feat(mp->m_log);
+
+ /*
+ * If log-assisted xattrs are already enabled, the caller can use the
+ * log assisted swap functions with the log-incompat reference we got.
+ */
+ if (xfs_sb_version_haslogxattrs(&mp->m_sb))
+ return 0;
+
+ /* Enable log-assisted xattrs. */
+ error = xfs_add_incompat_log_feature(mp,
+ XFS_SB_FEAT_INCOMPAT_LOG_XATTRS);
+ if (error)
+ goto drop_incompat;
+
+ xfs_warn_mount(mp, XFS_OPSTATE_WARNED_LARP,
+ "EXPERIMENTAL logged extended attributes feature in use. Use at your own risk!");
+
+ return 0;
+drop_incompat:
+ xlog_drop_incompat_feat(mp->m_log);
+ return error;
+}
+
+void
+xfs_attr_rele_log_assist(
+ struct xfs_mount *mp)
+{
+ xlog_drop_incompat_feat(mp->m_log);
+}
static int
xfs_xattr_get(const struct xattr_handler *handler, struct dentry *unused,
diff --git a/fs/xfs/xfs_xattr.h b/fs/xfs/xfs_xattr.h
new file mode 100644
index 000000000000..d34ef1835541
--- /dev/null
+++ b/fs/xfs/xfs_xattr.h
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000-2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ */
+#ifndef __XFS_XATTR_H__
+#define __XFS_XATTR_H__
+
+int xfs_attr_grab_log_assist(struct xfs_mount *mp);
+void xfs_attr_rele_log_assist(struct xfs_mount *mp);
+
+extern const struct xattr_handler *xfs_xattr_handlers[];
+
+#endif /* __XFS_XATTR_H__ */
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/5] xfs: move xfs_attr_use_log_assist out of xfs_log.c
2022-05-24 5:36 ` [PATCH 4/5] xfs: move xfs_attr_use_log_assist out of xfs_log.c Darrick J. Wong
@ 2022-05-27 1:28 ` Dave Chinner
0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2022-05-27 1:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, allison.henderson
On Mon, May 23, 2022 at 10:36:42PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> The LARP patchset added an awkward coupling point between libxfs and
> what would be libxlog, if the XFS log were actually its own library.
> Move the code that enables logged xattr updates out of "lib"xlog and into
> xfs_xattr.c so that it no longer has to know about xlog_* functions.
>
> While we're at it, give xfs_xattr.c its own header file.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_attr.c | 6 +++---
> fs/xfs/xfs_log.c | 41 --------------------------------------
> fs/xfs/xfs_super.c | 1 +
> fs/xfs/xfs_super.h | 1 -
> fs/xfs/xfs_xattr.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_xattr.h | 14 +++++++++++++
> 6 files changed, 67 insertions(+), 45 deletions(-)
> create mode 100644 fs/xfs/xfs_xattr.h
fs/xfs/xfs_xattr.c was the only place I could really think
that this kinda fitted, too.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] xfs: move xfs_attr_use_log_assist usage out of libxfs
2022-05-24 5:36 [PATCHSET v2 0/5] xfs: last pile of LARP cleanups for 5.19 Darrick J. Wong
` (3 preceding siblings ...)
2022-05-24 5:36 ` [PATCH 4/5] xfs: move xfs_attr_use_log_assist out of xfs_log.c Darrick J. Wong
@ 2022-05-24 5:36 ` Darrick J. Wong
2022-05-27 1:29 ` Dave Chinner
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-05-24 5:36 UTC (permalink / raw)
To: djwong; +Cc: linux-xfs, david, allison.henderson
From: Darrick J. Wong <djwong@kernel.org>
The LARP patchset added an awkward coupling point between libxfs and
what would be libxlog, if the XFS log were actually its own library.
Move the code that sets up logged xattr updates out of libxfs and into
xfs_xattr.c so that libxfs no longer has to know about xlog_* functions.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr.c | 12 +-----------
fs/xfs/xfs_acl.c | 3 ++-
fs/xfs/xfs_ioctl.c | 3 ++-
fs/xfs/xfs_iops.c | 3 ++-
fs/xfs/xfs_xattr.c | 34 +++++++++++++++++++++++++++++++---
fs/xfs/xfs_xattr.h | 3 +--
6 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 24fa213715c1..836ab1b8ed7b 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -982,7 +982,6 @@ xfs_attr_set(
int error, local;
int rmt_blks = 0;
unsigned int total;
- bool use_logging = xfs_has_larp(mp);
if (xfs_is_shutdown(dp->i_mount))
return -EIO;
@@ -1027,12 +1026,6 @@ xfs_attr_set(
rmt_blks = xfs_attr3_rmt_blocks(mp, XFS_XATTR_SIZE_MAX);
}
- if (use_logging) {
- error = xfs_attr_grab_log_assist(mp);
- if (error)
- return error;
- }
-
/*
* Root fork attributes can use reserved data blocks for this
* operation if necessary
@@ -1040,7 +1033,7 @@ xfs_attr_set(
xfs_init_attr_trans(args, &tres, &total);
error = xfs_trans_alloc_inode(dp, &tres, total, 0, rsvd, &args->trans);
if (error)
- goto drop_incompat;
+ return error;
if (args->value || xfs_inode_hasattr(dp)) {
error = xfs_iext_count_may_overflow(dp, XFS_ATTR_FORK,
@@ -1100,9 +1093,6 @@ xfs_attr_set(
error = xfs_trans_commit(args->trans);
out_unlock:
xfs_iunlock(dp, XFS_ILOCK_EXCL);
-drop_incompat:
- if (use_logging)
- xfs_attr_rele_log_assist(mp);
return error;
out_trans_cancel:
diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
index 3df9c1782ead..b744c62052b6 100644
--- a/fs/xfs/xfs_acl.c
+++ b/fs/xfs/xfs_acl.c
@@ -17,6 +17,7 @@
#include "xfs_error.h"
#include "xfs_acl.h"
#include "xfs_trans.h"
+#include "xfs_xattr.h"
#include <linux/posix_acl_xattr.h>
@@ -202,7 +203,7 @@ __xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
xfs_acl_to_disk(args.value, acl);
}
- error = xfs_attr_set(&args);
+ error = xfs_attr_change(&args);
kmem_free(args.value);
/*
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 0e5cb7936206..5a364a7d58fd 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -37,6 +37,7 @@
#include "xfs_health.h"
#include "xfs_reflink.h"
#include "xfs_ioctl.h"
+#include "xfs_xattr.h"
#include <linux/mount.h>
#include <linux/namei.h>
@@ -524,7 +525,7 @@ xfs_attrmulti_attr_set(
args.valuelen = len;
}
- error = xfs_attr_set(&args);
+ error = xfs_attr_change(&args);
if (!error && (flags & XFS_IOC_ATTR_ROOT))
xfs_forget_acl(inode, name);
kfree(args.value);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index e912b7fee714..29f5b8b8aca6 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -24,6 +24,7 @@
#include "xfs_iomap.h"
#include "xfs_error.h"
#include "xfs_ioctl.h"
+#include "xfs_xattr.h"
#include <linux/posix_acl.h>
#include <linux/security.h>
@@ -61,7 +62,7 @@ xfs_initxattrs(
.value = xattr->value,
.valuelen = xattr->value_len,
};
- error = xfs_attr_set(&args);
+ error = xfs_attr_change(&args);
if (error < 0)
break;
}
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index fc6acf7021a7..35e13e125ec6 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -27,7 +27,7 @@
* they must release the permission by calling xlog_drop_incompat_feat
* when they're done.
*/
-int
+static inline int
xfs_attr_grab_log_assist(
struct xfs_mount *mp)
{
@@ -61,13 +61,41 @@ xfs_attr_grab_log_assist(
return error;
}
-void
+static inline void
xfs_attr_rele_log_assist(
struct xfs_mount *mp)
{
xlog_drop_incompat_feat(mp->m_log);
}
+/*
+ * Set or remove an xattr, having grabbed the appropriate logging resources
+ * prior to calling libxfs.
+ */
+int
+xfs_attr_change(
+ struct xfs_da_args *args)
+{
+ struct xfs_mount *mp = args->dp->i_mount;
+ bool use_logging = false;
+ int error;
+
+ if (xfs_has_larp(mp)) {
+ error = xfs_attr_grab_log_assist(mp);
+ if (error)
+ return error;
+
+ use_logging = true;
+ }
+
+ error = xfs_attr_set(args);
+
+ if (use_logging)
+ xfs_attr_rele_log_assist(mp);
+ return error;
+}
+
+
static int
xfs_xattr_get(const struct xattr_handler *handler, struct dentry *unused,
struct inode *inode, const char *name, void *value, size_t size)
@@ -105,7 +133,7 @@ xfs_xattr_set(const struct xattr_handler *handler,
};
int error;
- error = xfs_attr_set(&args);
+ error = xfs_attr_change(&args);
if (!error && (handler->flags & XFS_ATTR_ROOT))
xfs_forget_acl(inode, name);
return error;
diff --git a/fs/xfs/xfs_xattr.h b/fs/xfs/xfs_xattr.h
index d34ef1835541..2b09133b1b9b 100644
--- a/fs/xfs/xfs_xattr.h
+++ b/fs/xfs/xfs_xattr.h
@@ -6,8 +6,7 @@
#ifndef __XFS_XATTR_H__
#define __XFS_XATTR_H__
-int xfs_attr_grab_log_assist(struct xfs_mount *mp);
-void xfs_attr_rele_log_assist(struct xfs_mount *mp);
+int xfs_attr_change(struct xfs_da_args *args);
extern const struct xattr_handler *xfs_xattr_handlers[];
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] xfs: move xfs_attr_use_log_assist usage out of libxfs
2022-05-24 5:36 ` [PATCH 5/5] xfs: move xfs_attr_use_log_assist usage out of libxfs Darrick J. Wong
@ 2022-05-27 1:29 ` Dave Chinner
0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2022-05-27 1:29 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, allison.henderson
On Mon, May 23, 2022 at 10:36:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> The LARP patchset added an awkward coupling point between libxfs and
> what would be libxlog, if the XFS log were actually its own library.
> Move the code that sets up logged xattr updates out of libxfs and into
> xfs_xattr.c so that libxfs no longer has to know about xlog_* functions.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_attr.c | 12 +-----------
> fs/xfs/xfs_acl.c | 3 ++-
> fs/xfs/xfs_ioctl.c | 3 ++-
> fs/xfs/xfs_iops.c | 3 ++-
> fs/xfs/xfs_xattr.c | 34 +++++++++++++++++++++++++++++++---
> fs/xfs/xfs_xattr.h | 3 +--
> 6 files changed, 39 insertions(+), 19 deletions(-)
That makes the libxfs side of things much neater.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread