* [PATCH] xfs: restore speculative_cow_prealloc_lifetime sysctl
@ 2021-02-12 17:24 Darrick J. Wong
2021-02-12 21:21 ` Dave Chinner
2021-02-12 21:48 ` [PATCH v2] " Darrick J. Wong
0 siblings, 2 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-02-12 17:24 UTC (permalink / raw)
To: xfs; +Cc: Dave Chinner
From: Darrick J. Wong <djwong@kernel.org>
In commit 9669f51de5c0 I tried to get rid of the undocumented cow gc
lifetime knob. The knob's function was never documented and it now
doesn't really have a function since eof and cow gc have been
consolidated.
Regrettably, xfs/231 relies on it and regresses on for-next. I did not
succeed at getting far enough through fstests patch review for the fixup
to land in time.
Restore the sysctl knob, document what it did (does?), put it on the
deprecation schedule, and rip out a redundant function.
Fixes: 9669f51de5c0 ("xfs: consolidate the eofblocks and cowblocks workers")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
Documentation/admin-guide/xfs.rst | 4 ++++
fs/xfs/xfs_sysctl.c | 33 +++++++++++++--------------------
2 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
index 6178153d3320..e188d0ea7b5b 100644
--- a/Documentation/admin-guide/xfs.rst
+++ b/Documentation/admin-guide/xfs.rst
@@ -284,6 +284,9 @@ The following sysctls are available for the XFS filesystem:
removes unused preallocation from clean inodes and releases
the unused space back to the free pool.
+ fs.xfs.speculative_cow_prealloc_lifetime
+ This is an alias for speculative_prealloc_lifetime.
+
fs.xfs.error_level (Min: 0 Default: 3 Max: 11)
A volume knob for error reporting when internal errors occur.
This will generate detailed messages & backtraces for filesystem
@@ -361,6 +364,7 @@ Deprecated Sysctls
=========================== ================
fs.xfs.irix_sgid_inherit September 2025
fs.xfs.irix_symlink_mode September 2025
+fs.xfs.speculative_cow_prealloc_lifetime September 2025
=========================== ================
diff --git a/fs/xfs/xfs_sysctl.c b/fs/xfs/xfs_sysctl.c
index 145e06c47744..75a3c7f0db8e 100644
--- a/fs/xfs/xfs_sysctl.c
+++ b/fs/xfs/xfs_sysctl.c
@@ -51,7 +51,7 @@ xfs_panic_mask_proc_handler(
#endif /* CONFIG_PROC_FS */
STATIC int
-xfs_deprecate_irix_sgid_inherit_proc_handler(
+xfs_deprecated_dointvec_minmax(
struct ctl_table *ctl,
int write,
void *buffer,
@@ -60,23 +60,7 @@ xfs_deprecate_irix_sgid_inherit_proc_handler(
{
if (write) {
printk_once(KERN_WARNING
- "XFS: " "%s sysctl option is deprecated.\n",
- ctl->procname);
- }
- return proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
-}
-
-STATIC int
-xfs_deprecate_irix_symlink_mode_proc_handler(
- struct ctl_table *ctl,
- int write,
- void *buffer,
- size_t *lenp,
- loff_t *ppos)
-{
- if (write) {
- printk_once(KERN_WARNING
- "XFS: " "%s sysctl option is deprecated.\n",
+ "XFS: %s sysctl option is deprecated.\n",
ctl->procname);
}
return proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
@@ -88,7 +72,7 @@ static struct ctl_table xfs_table[] = {
.data = &xfs_params.sgid_inherit.val,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = xfs_deprecate_irix_sgid_inherit_proc_handler,
+ .proc_handler = xfs_deprecated_dointvec_minmax,
.extra1 = &xfs_params.sgid_inherit.min,
.extra2 = &xfs_params.sgid_inherit.max
},
@@ -97,7 +81,7 @@ static struct ctl_table xfs_table[] = {
.data = &xfs_params.symlink_mode.val,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = xfs_deprecate_irix_symlink_mode_proc_handler,
+ .proc_handler = xfs_deprecated_dointvec_minmax,
.extra1 = &xfs_params.symlink_mode.min,
.extra2 = &xfs_params.symlink_mode.max
},
@@ -201,6 +185,15 @@ static struct ctl_table xfs_table[] = {
.extra1 = &xfs_params.blockgc_timer.min,
.extra2 = &xfs_params.blockgc_timer.max,
},
+ {
+ .procname = "speculative_cow_prealloc_lifetime",
+ .data = &xfs_params.blockgc_timer.val,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = xfs_deprecated_dointvec_minmax,
+ .extra1 = &xfs_params.blockgc_timer.min,
+ .extra2 = &xfs_params.blockgc_timer.max,
+ },
/* please keep this the last entry */
#ifdef CONFIG_PROC_FS
{
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: restore speculative_cow_prealloc_lifetime sysctl
2021-02-12 17:24 [PATCH] xfs: restore speculative_cow_prealloc_lifetime sysctl Darrick J. Wong
@ 2021-02-12 21:21 ` Dave Chinner
2021-02-12 21:28 ` Darrick J. Wong
2021-02-12 21:48 ` [PATCH v2] " Darrick J. Wong
1 sibling, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2021-02-12 21:21 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On Fri, Feb 12, 2021 at 09:24:36AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> In commit 9669f51de5c0 I tried to get rid of the undocumented cow gc
> lifetime knob. The knob's function was never documented and it now
> doesn't really have a function since eof and cow gc have been
> consolidated.
>
> Regrettably, xfs/231 relies on it and regresses on for-next. I did not
> succeed at getting far enough through fstests patch review for the fixup
> to land in time.
>
> Restore the sysctl knob, document what it did (does?), put it on the
> deprecation schedule, and rip out a redundant function.
>
> Fixes: 9669f51de5c0 ("xfs: consolidate the eofblocks and cowblocks workers")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Thnanks for doing this, Darrick!
> STATIC int
> -xfs_deprecate_irix_sgid_inherit_proc_handler(
> +xfs_deprecated_dointvec_minmax(
> struct ctl_table *ctl,
> int write,
> void *buffer,
> @@ -60,23 +60,7 @@ xfs_deprecate_irix_sgid_inherit_proc_handler(
> {
> if (write) {
> printk_once(KERN_WARNING
> - "XFS: " "%s sysctl option is deprecated.\n",
> - ctl->procname);
> - }
> - return proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
> -}
> -
> -STATIC int
> -xfs_deprecate_irix_symlink_mode_proc_handler(
> - struct ctl_table *ctl,
> - int write,
> - void *buffer,
> - size_t *lenp,
> - loff_t *ppos)
> -{
> - if (write) {
> - printk_once(KERN_WARNING
> - "XFS: " "%s sysctl option is deprecated.\n",
> + "XFS: %s sysctl option is deprecated.\n",
> ctl->procname);
> }
The use of printk_once means it will only warn on the first
deprecated sysctl written to, not the first write to each of the
deprecated sysctls.
Is there any evidence that anyone is writing these with any regualr
frequency, and if not, maybe just a ratelimited warning is
sufficient here?
Otherwise looks fine.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: restore speculative_cow_prealloc_lifetime sysctl
2021-02-12 21:21 ` Dave Chinner
@ 2021-02-12 21:28 ` Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-02-12 21:28 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On Sat, Feb 13, 2021 at 08:21:47AM +1100, Dave Chinner wrote:
> On Fri, Feb 12, 2021 at 09:24:36AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > In commit 9669f51de5c0 I tried to get rid of the undocumented cow gc
> > lifetime knob. The knob's function was never documented and it now
> > doesn't really have a function since eof and cow gc have been
> > consolidated.
> >
> > Regrettably, xfs/231 relies on it and regresses on for-next. I did not
> > succeed at getting far enough through fstests patch review for the fixup
> > to land in time.
> >
> > Restore the sysctl knob, document what it did (does?), put it on the
> > deprecation schedule, and rip out a redundant function.
> >
> > Fixes: 9669f51de5c0 ("xfs: consolidate the eofblocks and cowblocks workers")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
>
> Thnanks for doing this, Darrick!
>
> > STATIC int
> > -xfs_deprecate_irix_sgid_inherit_proc_handler(
> > +xfs_deprecated_dointvec_minmax(
> > struct ctl_table *ctl,
> > int write,
> > void *buffer,
> > @@ -60,23 +60,7 @@ xfs_deprecate_irix_sgid_inherit_proc_handler(
> > {
> > if (write) {
> > printk_once(KERN_WARNING
> > - "XFS: " "%s sysctl option is deprecated.\n",
> > - ctl->procname);
> > - }
> > - return proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
> > -}
> > -
> > -STATIC int
> > -xfs_deprecate_irix_symlink_mode_proc_handler(
> > - struct ctl_table *ctl,
> > - int write,
> > - void *buffer,
> > - size_t *lenp,
> > - loff_t *ppos)
> > -{
> > - if (write) {
> > - printk_once(KERN_WARNING
> > - "XFS: " "%s sysctl option is deprecated.\n",
> > + "XFS: %s sysctl option is deprecated.\n",
> > ctl->procname);
> > }
>
> The use of printk_once means it will only warn on the first
> deprecated sysctl written to, not the first write to each of the
> deprecated sysctls.
>
> Is there any evidence that anyone is writing these with any regualr
> frequency, and if not, maybe just a ratelimited warning is
> sufficient here?
I've never seen any evidence that people hammer on sysctls with great
frequency. Will change to printk_ratelimited.
--D
> Otherwise looks fine.
>
> Cheers,
>
> Dave.
>
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] xfs: restore speculative_cow_prealloc_lifetime sysctl
2021-02-12 17:24 [PATCH] xfs: restore speculative_cow_prealloc_lifetime sysctl Darrick J. Wong
2021-02-12 21:21 ` Dave Chinner
@ 2021-02-12 21:48 ` Darrick J. Wong
2021-02-12 22:46 ` Dave Chinner
2021-02-25 7:37 ` Christoph Hellwig
1 sibling, 2 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-02-12 21:48 UTC (permalink / raw)
To: xfs; +Cc: Dave Chinner
From: Darrick J. Wong <djwong@kernel.org>
In commit 9669f51de5c0 I tried to get rid of the undocumented cow gc
lifetime knob. The knob's function was never documented and it now
doesn't really have a function since eof and cow gc have been
consolidated.
Regrettably, xfs/231 relies on it and regresses on for-next. I did not
succeed at getting far enough through fstests patch review for the fixup
to land in time.
Restore the sysctl knob, document what it did (does?), put it on the
deprecation schedule, and rip out a redundant function.
Fixes: 9669f51de5c0 ("xfs: consolidate the eofblocks and cowblocks workers")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
v2: use printk_ratelimited
---
Documentation/admin-guide/xfs.rst | 4 ++++
fs/xfs/xfs_sysctl.c | 35 ++++++++++++++---------------------
2 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
index 6178153d3320..e188d0ea7b5b 100644
--- a/Documentation/admin-guide/xfs.rst
+++ b/Documentation/admin-guide/xfs.rst
@@ -284,6 +284,9 @@ The following sysctls are available for the XFS filesystem:
removes unused preallocation from clean inodes and releases
the unused space back to the free pool.
+ fs.xfs.speculative_cow_prealloc_lifetime
+ This is an alias for speculative_prealloc_lifetime.
+
fs.xfs.error_level (Min: 0 Default: 3 Max: 11)
A volume knob for error reporting when internal errors occur.
This will generate detailed messages & backtraces for filesystem
@@ -361,6 +364,7 @@ Deprecated Sysctls
=========================== ================
fs.xfs.irix_sgid_inherit September 2025
fs.xfs.irix_symlink_mode September 2025
+fs.xfs.speculative_cow_prealloc_lifetime September 2025
=========================== ================
diff --git a/fs/xfs/xfs_sysctl.c b/fs/xfs/xfs_sysctl.c
index 145e06c47744..546a6cd96729 100644
--- a/fs/xfs/xfs_sysctl.c
+++ b/fs/xfs/xfs_sysctl.c
@@ -51,7 +51,7 @@ xfs_panic_mask_proc_handler(
#endif /* CONFIG_PROC_FS */
STATIC int
-xfs_deprecate_irix_sgid_inherit_proc_handler(
+xfs_deprecated_dointvec_minmax(
struct ctl_table *ctl,
int write,
void *buffer,
@@ -59,24 +59,8 @@ xfs_deprecate_irix_sgid_inherit_proc_handler(
loff_t *ppos)
{
if (write) {
- printk_once(KERN_WARNING
- "XFS: " "%s sysctl option is deprecated.\n",
- ctl->procname);
- }
- return proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
-}
-
-STATIC int
-xfs_deprecate_irix_symlink_mode_proc_handler(
- struct ctl_table *ctl,
- int write,
- void *buffer,
- size_t *lenp,
- loff_t *ppos)
-{
- if (write) {
- printk_once(KERN_WARNING
- "XFS: " "%s sysctl option is deprecated.\n",
+ printk_ratelimited(KERN_WARNING
+ "XFS: %s sysctl option is deprecated.\n",
ctl->procname);
}
return proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
@@ -88,7 +72,7 @@ static struct ctl_table xfs_table[] = {
.data = &xfs_params.sgid_inherit.val,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = xfs_deprecate_irix_sgid_inherit_proc_handler,
+ .proc_handler = xfs_deprecated_dointvec_minmax,
.extra1 = &xfs_params.sgid_inherit.min,
.extra2 = &xfs_params.sgid_inherit.max
},
@@ -97,7 +81,7 @@ static struct ctl_table xfs_table[] = {
.data = &xfs_params.symlink_mode.val,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = xfs_deprecate_irix_symlink_mode_proc_handler,
+ .proc_handler = xfs_deprecated_dointvec_minmax,
.extra1 = &xfs_params.symlink_mode.min,
.extra2 = &xfs_params.symlink_mode.max
},
@@ -201,6 +185,15 @@ static struct ctl_table xfs_table[] = {
.extra1 = &xfs_params.blockgc_timer.min,
.extra2 = &xfs_params.blockgc_timer.max,
},
+ {
+ .procname = "speculative_cow_prealloc_lifetime",
+ .data = &xfs_params.blockgc_timer.val,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = xfs_deprecated_dointvec_minmax,
+ .extra1 = &xfs_params.blockgc_timer.min,
+ .extra2 = &xfs_params.blockgc_timer.max,
+ },
/* please keep this the last entry */
#ifdef CONFIG_PROC_FS
{
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] xfs: restore speculative_cow_prealloc_lifetime sysctl
2021-02-12 21:48 ` [PATCH v2] " Darrick J. Wong
@ 2021-02-12 22:46 ` Dave Chinner
2021-02-25 7:37 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2021-02-12 22:46 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On Fri, Feb 12, 2021 at 01:48:02PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> In commit 9669f51de5c0 I tried to get rid of the undocumented cow gc
> lifetime knob. The knob's function was never documented and it now
> doesn't really have a function since eof and cow gc have been
> consolidated.
>
> Regrettably, xfs/231 relies on it and regresses on for-next. I did not
> succeed at getting far enough through fstests patch review for the fixup
> to land in time.
>
> Restore the sysctl knob, document what it did (does?), put it on the
> deprecation schedule, and rip out a redundant function.
>
> Fixes: 9669f51de5c0 ("xfs: consolidate the eofblocks and cowblocks workers")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> v2: use printk_ratelimited
> ---
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] xfs: restore speculative_cow_prealloc_lifetime sysctl
2021-02-12 21:48 ` [PATCH v2] " Darrick J. Wong
2021-02-12 22:46 ` Dave Chinner
@ 2021-02-25 7:37 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2021-02-25 7:37 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs, Dave Chinner
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-25 7:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-12 17:24 [PATCH] xfs: restore speculative_cow_prealloc_lifetime sysctl Darrick J. Wong
2021-02-12 21:21 ` Dave Chinner
2021-02-12 21:28 ` Darrick J. Wong
2021-02-12 21:48 ` [PATCH v2] " Darrick J. Wong
2021-02-12 22:46 ` Dave Chinner
2021-02-25 7:37 ` Christoph Hellwig
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).