On 9/20/22 14:11, Alexander Atanasov wrote:
> In (060807f841ac mm, slub: make remaining slub_debug related attributes
> read-only) failslab was made read-only.
> I think it became a collateral victim to the two other options for which
> the reasons are perfectly valid.
> Here is why:
> - sanity_checks and trace are slab internal debug options,
> failslab is used for fault injection.
> - for fault injections, which by presumption are random, it
> does not matter if it is not set atomically. And you need to
> set atleast one more option to trigger fault injection.
> - in a testing scenario you may need to change it at runtime
> example: module loading - you test all allocations limited
> by the space option. Then you move to test only your module's
> own slabs.
> - when set by command line flags it effectively disables all
> cache merges.
>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> Cc: Christoph Lameter <[email protected]>
> Cc: Jann Horn <[email protected]>
> Cc: Vijayanand Jitta <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: Joonsoo Kim <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Link: http://lkml.kernel.org/r/[email protected]
>
> Signed-off-by: Alexander Atanasov <[email protected]>
Thanks, added to slab.git for-6.2/slub-sysfs
> ---
> Documentation/mm/slub.rst | 2 ++
> mm/slub.c | 16 +++++++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> V1->V2: Fixed commit message. Flags are set using WRITE_ONCE.
>
> diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst
> index 43063ade737a..86837073a39e 100644
> --- a/Documentation/mm/slub.rst
> +++ b/Documentation/mm/slub.rst
> @@ -116,6 +116,8 @@ options from the ``slub_debug`` parameter translate to the following files::
> T trace
> A failslab
>
> +failslab file is writable, so writing 1 or 0 will enable or disable
> +the option at runtime. Write returns -EINVAL if cache is an alias.
> Careful with tracing: It may spew out lots of information and never stop if
> used on the wrong slab.
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 862dbd9af4f5..57cf18936526 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5617,7 +5617,21 @@ static ssize_t failslab_show(struct kmem_cache *s, char *buf)
> {
> return sysfs_emit(buf, "%d\n", !!(s->flags & SLAB_FAILSLAB));
> }
> -SLAB_ATTR_RO(failslab);
> +
> +static ssize_t failslab_store(struct kmem_cache *s, const char *buf,
> + size_t length)
> +{
> + if (s->refcount > 1)
> + return -EINVAL;
> +
> + if (buf[0] == '1')
> + WRITE_ONCE(s->flags, s->flags | SLAB_FAILSLAB);
> + else
> + WRITE_ONCE(s->flags, s->flags & ~SLAB_FAILSLAB);
> +
> + return length;
> +}
> +SLAB_ATTR(failslab);
> #endif
>
> static ssize_t shrink_show(struct kmem_cache *s, char *buf)
>
> base-commit: 80e78fcce86de0288793a0ef0f6acf37656ee4cf