Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] failslab: add abitily to filter slab caches

6 views
Skip to first unread message

Dmitry Monakhov

unread,
Feb 23, 2010, 8:40:01 AM2/23/10
to
Example:
echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
echo 1 > /sys/kernel/debug/failslab/cache-filter

Signed-off-by: Dmitry Monakhov <dmon...@openvz.org>
---
include/linux/fault-inject.h | 4 ++--
include/linux/slab.h | 4 +++-
mm/failslab.c | 18 +++++++++++++++---
mm/slab.c | 2 +-
mm/slub.c | 31 +++++++++++++++++++++++++++++--
5 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h
index 06ca9b2..d935647 100644
--- a/include/linux/fault-inject.h
+++ b/include/linux/fault-inject.h
@@ -82,9 +82,9 @@ static inline void cleanup_fault_attr_dentries(struct fault_attr *attr)
#endif /* CONFIG_FAULT_INJECTION */

#ifdef CONFIG_FAILSLAB
-extern bool should_failslab(size_t size, gfp_t gfpflags);
+extern bool should_failslab(size_t size, gfp_t gfpflags, unsigned long c_flags);
#else
-static inline bool should_failslab(size_t size, gfp_t gfpflags)
+static inline bool should_failslab(size_t size, gfp_t gfpflags, unsigned long f)
{
return false;
}
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 2da8372..9e03a81 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -70,7 +70,9 @@
#else
# define SLAB_NOTRACK 0x00000000UL
#endif
-
+#ifdef CONFIG_FAILSLAB
+# define SLAB_FAILSLAB 0x02000000UL /* Fault injection filter mark */
+#endif
/* The following flags affect the page allocator grouping pages by mobility */
#define SLAB_RECLAIM_ACCOUNT 0x00020000UL /* Objects are reclaimable */
#define SLAB_TEMPORARY SLAB_RECLAIM_ACCOUNT /* Objects are short-lived */
diff --git a/mm/failslab.c b/mm/failslab.c
index 9339de5..bb41f98 100644
--- a/mm/failslab.c
+++ b/mm/failslab.c
@@ -1,18 +1,22 @@
#include <linux/fault-inject.h>
#include <linux/gfp.h>
+#include <linux/slab.h>

static struct {
struct fault_attr attr;
u32 ignore_gfp_wait;
+ int cache_filter;
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
struct dentry *ignore_gfp_wait_file;
+ struct dentry *cache_filter_file;
#endif
} failslab = {
.attr = FAULT_ATTR_INITIALIZER,
.ignore_gfp_wait = 1,
+ .cache_filter = 0,
};

-bool should_failslab(size_t size, gfp_t gfpflags)
+bool should_failslab(size_t size, gfp_t gfpflags, unsigned long cache_flags)
{
if (gfpflags & __GFP_NOFAIL)
return false;
@@ -20,6 +24,9 @@ bool should_failslab(size_t size, gfp_t gfpflags)
if (failslab.ignore_gfp_wait && (gfpflags & __GFP_WAIT))
return false;

+ if (failslab.cache_filter && !(cache_flags & SLAB_FAILSLAB))
+ return false;
+
return should_fail(&failslab.attr, size);
}

@@ -30,7 +37,6 @@ static int __init setup_failslab(char *str)
__setup("failslab=", setup_failslab);

#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
-
static int __init failslab_debugfs_init(void)
{
mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
@@ -46,8 +52,14 @@ static int __init failslab_debugfs_init(void)
debugfs_create_bool("ignore-gfp-wait", mode, dir,
&failslab.ignore_gfp_wait);

- if (!failslab.ignore_gfp_wait_file) {
+ failslab.cache_filter_file =
+ debugfs_create_bool("cache-filter", mode, dir,
+ &failslab.cache_filter);
+
+ if (!failslab.ignore_gfp_wait_file ||
+ !failslab.cache_filter_file) {
err = -ENOMEM;
+ debugfs_remove(failslab.cache_filter_file);
debugfs_remove(failslab.ignore_gfp_wait_file);
cleanup_fault_attr_dentries(&failslab.attr);
}
diff --git a/mm/slab.c b/mm/slab.c
index 7451bda..33496b7 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3101,7 +3101,7 @@ static bool slab_should_failslab(struct kmem_cache *cachep, gfp_t flags)
if (cachep == &cache_cache)
return false;

- return should_failslab(obj_size(cachep), flags);
+ return should_failslab(obj_size(cachep), flags, cachep->flags);
}

static inline void *____cache_alloc(struct kmem_cache *cachep, gfp_t flags)
diff --git a/mm/slub.c b/mm/slub.c
index 8d71aaf..64114fa 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -151,7 +151,8 @@
* Set of flags that will prevent slab merging
*/
#define SLUB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
- SLAB_TRACE | SLAB_DESTROY_BY_RCU | SLAB_NOLEAKTRACE)
+ SLAB_TRACE | SLAB_DESTROY_BY_RCU | SLAB_NOLEAKTRACE | \
+ SLAB_FAILSLAB)

#define SLUB_MERGE_SAME (SLAB_DEBUG_FREE | SLAB_RECLAIM_ACCOUNT | \
SLAB_CACHE_DMA | SLAB_NOTRACK)
@@ -1020,6 +1021,11 @@ static int __init setup_slub_debug(char *str)
case 't':
slub_debug |= SLAB_TRACE;
break;
+#ifdef CONFIG_FAILSLAB
+ case 'a':
+ slub_debug |= SLAB_FAILSLAB;
+ break;
+#endif
default:
printk(KERN_ERR "slub_debug option '%c' "
"unknown. skipped\n", *str);
@@ -1718,7 +1724,7 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
lockdep_trace_alloc(gfpflags);
might_sleep_if(gfpflags & __GFP_WAIT);

- if (should_failslab(s->objsize, gfpflags))
+ if (should_failslab(s->objsize, gfpflags, s->flags))
return NULL;

local_irq_save(flags);
@@ -4171,6 +4177,23 @@ static ssize_t trace_store(struct kmem_cache *s, const char *buf,
}
SLAB_ATTR(trace);

+#ifdef CONFIG_FAILSLAB
+static ssize_t failslab_show(struct kmem_cache *s, char *buf)
+{
+ return sprintf(buf, "%d\n", !!(s->flags & SLAB_FAILSLAB));
+}
+
+static ssize_t failslab_store(struct kmem_cache *s, const char *buf,
+ size_t length)
+{
+ s->flags &= ~SLAB_FAILSLAB;
+ if (buf[0] == '1')
+ s->flags |= SLAB_FAILSLAB;
+ return length;
+}
+SLAB_ATTR(failslab);
+#endif
+
static ssize_t reclaim_account_show(struct kmem_cache *s, char *buf)
{
return sprintf(buf, "%d\n", !!(s->flags & SLAB_RECLAIM_ACCOUNT));
@@ -4467,6 +4490,10 @@ static struct attribute *slab_attrs[] = {
&deactivate_remote_frees_attr.attr,
&order_fallback_attr.attr,
#endif
+#ifdef CONFIG_FAILSLAB
+ &failslab_attr.attr,
+#endif
+
NULL
};

--
1.6.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Akinobu Mita

unread,
Feb 23, 2010, 9:00:02 AM2/23/10
to
2010/2/23 Dmitry Monakhov <dmon...@openvz.org>:

> Example:
> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
> echo 1 > /sys/kernel/debug/failslab/cache-filter

Looks good.

.Should we also support clearing the flag by
"echo 0 > /sys/kernel/slab/skbuff_head_cache/failslab" ?

> +static ssize_t failslab_store(struct kmem_cache *s, const char *buf,
> + � � � � � � � � � � � � � � � � � � � � � � � � � � � size_t length)
> +{
> + � � � s->flags &= ~SLAB_FAILSLAB;
> + � � � if (buf[0] == '1')
> + � � � � � � � s->flags |= SLAB_FAILSLAB;
> + � � � return length;
> +}
> +SLAB_ATTR(failslab);

Dmitry Monakhov

unread,
Feb 23, 2010, 9:10:02 AM2/23/10
to
Akinobu Mita <akinob...@gmail.com> writes:

> 2010/2/23 Dmitry Monakhov <dmon...@openvz.org>:
>> Example:
>> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
>> echo 1 > /sys/kernel/debug/failslab/cache-filter
>
> Looks good.
>
> .Should we also support clearing the flag by
> "echo 0 > /sys/kernel/slab/skbuff_head_cache/failslab" ?

But it already does this, any output except "1" treated as '0'
I've snipped this from trace option.

Akinobu Mita

unread,
Feb 23, 2010, 9:10:02 AM2/23/10
to
2010/2/23 Dmitry Monakhov <dmon...@openvz.org>:

> Akinobu Mita <akinob...@gmail.com> writes:
>
>> 2010/2/23 Dmitry Monakhov <dmon...@openvz.org>:
>>> Example:
>>> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
>>> echo 1 > /sys/kernel/debug/failslab/cache-filter
>>
>> Looks good.
>>
>> .Should we also support clearing the flag by
>> "echo 0 > /sys/kernel/slab/skbuff_head_cache/failslab" ?
> But it already does this, any output except "1" treated as '0'
> I've snipped this from trace option.

Oops, you are right.

David Rientjes

unread,
Feb 23, 2010, 5:40:02 PM2/23/10
to
On Tue, 23 Feb 2010, Dmitry Monakhov wrote:

> Example:
> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
> echo 1 > /sys/kernel/debug/failslab/cache-filter
>

Where's the changelog?

> Signed-off-by: Dmitry Monakhov <dmon...@openvz.org>
> ---
> include/linux/fault-inject.h | 4 ++--
> include/linux/slab.h | 4 +++-
> mm/failslab.c | 18 +++++++++++++++---
> mm/slab.c | 2 +-
> mm/slub.c | 31 +++++++++++++++++++++++++++++--
> 5 files changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h
> index 06ca9b2..d935647 100644
> --- a/include/linux/fault-inject.h
> +++ b/include/linux/fault-inject.h
> @@ -82,9 +82,9 @@ static inline void cleanup_fault_attr_dentries(struct fault_attr *attr)
> #endif /* CONFIG_FAULT_INJECTION */
>
> #ifdef CONFIG_FAILSLAB
> -extern bool should_failslab(size_t size, gfp_t gfpflags);
> +extern bool should_failslab(size_t size, gfp_t gfpflags, unsigned long c_flags);
> #else
> -static inline bool should_failslab(size_t size, gfp_t gfpflags)
> +static inline bool should_failslab(size_t size, gfp_t gfpflags, unsigned long f)
> {
> return false;
> }

c_flags or f could be described better.

Does this work with CONFIG_FAILSLAB=n?

Dmitry Monakhov

unread,
Feb 24, 2010, 1:40:02 PM2/24/10
to
David Rientjes <rien...@google.com> writes:

> On Tue, 23 Feb 2010, Dmitry Monakhov wrote:
>
>> Example:
>> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
>> echo 1 > /sys/kernel/debug/failslab/cache-filter
>>
>
> Where's the changelog?

I've hoped than subject is enough. But if you want more verbose
changelog, take following version.

0001-failslab-add-abitily-to-filter-slab-caches.patch

David Rientjes

unread,
Feb 24, 2010, 4:30:02 PM2/24/10
to
On Wed, 24 Feb 2010, Dmitry Monakhov wrote:

> >> Example:
> >> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
> >> echo 1 > /sys/kernel/debug/failslab/cache-filter
> >>
> >
> > Where's the changelog?
> I've hoped than subject is enough. But if you want more verbose
> changelog, take following version.
>

Please do not attach patches to emails when sending to LKML, they are
preferred to be inline. See Documentation/SubmittingPatches.

> This patch allow to inject faults only for specific slabs.
> In order to preserve default behavior cache filter is off by
> default (all caches are faulty).
>
> One may define specific set of slabs like this:
> # mark skbuff_head_cache as faulty
> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
> # Turn on cache filter (off by default)
> echo 1 > /sys/kernel/debug/failslab/cache-filter
> # Turn on fault injection
> echo 1 > /sys/kernel/debug/failslab/times
> echo 1 > /sys/kernel/debug/failslab/probability
>

Please describe your new slub_debug=a option here as well as in an update
to Documentation/vm/slub.txt.

> Signed-off-by: Dmitry Monakhov <dmon...@openvz.org>
> ---
> include/linux/fault-inject.h | 4 ++--
> include/linux/slab.h | 4 +++-
> mm/failslab.c | 18 +++++++++++++++---
> mm/slab.c | 2 +-
> mm/slub.c | 31 +++++++++++++++++++++++++++++--
> 5 files changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h
> index 06ca9b2..d935647 100644
> --- a/include/linux/fault-inject.h
> +++ b/include/linux/fault-inject.h
> @@ -82,9 +82,9 @@ static inline void cleanup_fault_attr_dentries(struct fault_attr *attr)
> #endif /* CONFIG_FAULT_INJECTION */
>
> #ifdef CONFIG_FAILSLAB
> -extern bool should_failslab(size_t size, gfp_t gfpflags);
> +extern bool should_failslab(size_t size, gfp_t gfpflags, unsigned long c_flags);
> #else
> -static inline bool should_failslab(size_t size, gfp_t gfpflags)
> +static inline bool should_failslab(size_t size, gfp_t gfpflags, unsigned long f)
> {
> return false;
> }

As I said in the first review, these need to be more descriptive: 'flags'
would be better.

> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 2da8372..9e03a81 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -70,7 +70,9 @@
> #else
> # define SLAB_NOTRACK 0x00000000UL
> #endif
> -
> +#ifdef CONFIG_FAILSLAB
> +# define SLAB_FAILSLAB 0x02000000UL /* Fault injection filter mark */
> +#endif

Trailing whitespace on your #endif.

cachep->flags for CONFIG_SLAB is of type unsigned int and
should_failslab() takes an unsigned long.

> static inline void *____cache_alloc(struct kmem_cache *cachep, gfp_t flags)
> diff --git a/mm/slub.c b/mm/slub.c
> index 8d71aaf..64114fa 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -151,7 +151,8 @@
> * Set of flags that will prevent slab merging
> */
> #define SLUB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
> - SLAB_TRACE | SLAB_DESTROY_BY_RCU | SLAB_NOLEAKTRACE)
> + SLAB_TRACE | SLAB_DESTROY_BY_RCU | SLAB_NOLEAKTRACE | \
> + SLAB_FAILSLAB)
>

> #define SLUB_MERGE_SAME (SLAB_DEBUG_FREE | SLAB_RECLAIM_ACCOUNT | \
> SLAB_CACHE_DMA | SLAB_NOTRACK)

As I mentioned in my first review of your patch, this breaks for
CONFIG_FAILSLAB=n. Please make sure your patch compiles for the
defconfig.

Dmitry Monakhov

unread,
Feb 25, 2010, 1:00:02 AM2/25/10
to
David Rientjes <rien...@google.com> writes:

> On Wed, 24 Feb 2010, Dmitry Monakhov wrote:
>
>> >> Example:
>> >> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
>> >> echo 1 > /sys/kernel/debug/failslab/cache-filter
>> >>
>> >
>> > Where's the changelog?
>> I've hoped than subject is enough. But if you want more verbose
>> changelog, take following version.
>>

Ohh... i'm sorry i've missed other comments in your previous review.
It will be fixed in second version. Also see major note about
slab flags later in the text.


>
> Please do not attach patches to emails when sending to LKML, they are
> preferred to be inline. See Documentation/SubmittingPatches.

Yeah, In fact i've chose inline option but my mailer by unknown reason
change it.


>
>> This patch allow to inject faults only for specific slabs.
>> In order to preserve default behavior cache filter is off by
>> default (all caches are faulty).
>>
>> One may define specific set of slabs like this:
>> # mark skbuff_head_cache as faulty
>> echo 1 > /sys/kernel/slab/skbuff_head_cache/failslab
>> # Turn on cache filter (off by default)
>> echo 1 > /sys/kernel/debug/failslab/cache-filter
>> # Turn on fault injection
>> echo 1 > /sys/kernel/debug/failslab/times
>> echo 1 > /sys/kernel/debug/failslab/probability
>>
>
> Please describe your new slub_debug=a option here as well as in an update
> to Documentation/vm/slub.txt.

Done


>
>> Signed-off-by: Dmitry Monakhov <dmon...@openvz.org>
>> ---
>> include/linux/fault-inject.h | 4 ++--
>> include/linux/slab.h | 4 +++-
>> mm/failslab.c | 18 +++++++++++++++---
>> mm/slab.c | 2 +-
>> mm/slub.c | 31 +++++++++++++++++++++++++++++--
>> 5 files changed, 50 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h
>> index 06ca9b2..d935647 100644
>> --- a/include/linux/fault-inject.h
>> +++ b/include/linux/fault-inject.h
>> @@ -82,9 +82,9 @@ static inline void cleanup_fault_attr_dentries(struct fault_attr *attr)
>> #endif /* CONFIG_FAULT_INJECTION */
>>
>> #ifdef CONFIG_FAILSLAB
>> -extern bool should_failslab(size_t size, gfp_t gfpflags);
>> +extern bool should_failslab(size_t size, gfp_t gfpflags, unsigned long c_flags);
>> #else
>> -static inline bool should_failslab(size_t size, gfp_t gfpflags)
>> +static inline bool should_failslab(size_t size, gfp_t gfpflags, unsigned long f)
>> {
>> return false;
>> }
>
> As I said in the first review, these need to be more descriptive: 'flags'
> would be better.

done


>
>> diff --git a/include/linux/slab.h b/include/linux/slab.h
>> index 2da8372..9e03a81 100644
>> --- a/include/linux/slab.h
>> +++ b/include/linux/slab.h
>> @@ -70,7 +70,9 @@
>> #else
>> # define SLAB_NOTRACK 0x00000000UL
>> #endif
>> -
>> +#ifdef CONFIG_FAILSLAB
>> +# define SLAB_FAILSLAB 0x02000000UL /* Fault injection filter mark */
>> +#endif
>
> Trailing whitespace on your #endif.

Sorry. fixed

Strange story, seems that flag definition in slab_def.h is differ
from slub_def.h and slob_def.h (unsigned long long)
Need to be fixed. Will send as separate patch.


>
>> static inline void *____cache_alloc(struct kmem_cache *cachep, gfp_t flags)
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 8d71aaf..64114fa 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -151,7 +151,8 @@
>> * Set of flags that will prevent slab merging
>> */
>> #define SLUB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
>> - SLAB_TRACE | SLAB_DESTROY_BY_RCU | SLAB_NOLEAKTRACE)
>> + SLAB_TRACE | SLAB_DESTROY_BY_RCU | SLAB_NOLEAKTRACE | \
>> + SLAB_FAILSLAB)
>>
>> #define SLUB_MERGE_SAME (SLAB_DEBUG_FREE | SLAB_RECLAIM_ACCOUNT | \
>> SLAB_CACHE_DMA | SLAB_NOTRACK)
>
> As I mentioned in my first review of your patch, this breaks for
> CONFIG_FAILSLAB=n. Please make sure your patch compiles for the
> defconfig.

fixed.

0 new messages