[PATCH] kprobe: increase kprobe_hash_table size - Kernel

This is a discussion on [PATCH] kprobe: increase kprobe_hash_table size - Kernel ; Increase the size of kprobe hash table to 512. It's useful when hundreds of kprobes were used in the kernel because current size is just 64. Signed-off-by: Masami Hiramatsu Acked-by: Ananth N Mavinakayanahalli --- kernel/kprobes.c | 2 +- 1 file ...

+ Reply to Thread
Results 1 to 8 of 8

Thread: [PATCH] kprobe: increase kprobe_hash_table size

  1. [PATCH] kprobe: increase kprobe_hash_table size

    Increase the size of kprobe hash table to 512. It's useful when hundreds
    of kprobes were used in the kernel because current size is just 64.

    Signed-off-by: Masami Hiramatsu
    Acked-by: Ananth N Mavinakayanahalli
    ---
    kernel/kprobes.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    Index: 2.6.28-rc3/kernel/kprobes.c
    ================================================== =================
    --- 2.6.28-rc3.orig/kernel/kprobes.c
    +++ 2.6.28-rc3/kernel/kprobes.c
    @@ -49,7 +49,7 @@
    #include
    #include

    -#define KPROBE_HASH_BITS 6
    +#define KPROBE_HASH_BITS 9
    #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)


    --
    Masami Hiramatsu

    Software Engineer
    Hitachi Computer Products (America) Inc.
    Software Solutions Division

    e-mail: mhiramat@redhat.com

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

  2. Re: [PATCH] kprobe: increase kprobe_hash_table size

    On Fri, 07 Nov 2008 18:44:30 -0500 Masami Hiramatsu wrote:

    > Increase the size of kprobe hash table to 512. It's useful when hundreds
    > of kprobes were used in the kernel because current size is just 64.
    >


    "useful" is a bit vague. How big is the problem which this solves, and
    how well did it solve it?

    See, someone (me) needs to decide whether to merge this and if so,
    whether to merge it into 2.6.29, 2.6.28, 2.6.27.x, 2.6.26.x and
    2.6.25.x. I'll need more information to make that decision, but I do
    not have it.

    > --- 2.6.28-rc3.orig/kernel/kprobes.c
    > +++ 2.6.28-rc3/kernel/kprobes.c
    > @@ -49,7 +49,7 @@
    > #include
    > #include
    >
    > -#define KPROBE_HASH_BITS 6
    > +#define KPROBE_HASH_BITS 9
    > #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)



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

  3. Re: [PATCH] kprobe: increase kprobe_hash_table size

    Hi Andrew,

    Andrew Morton wrote:
    > On Fri, 07 Nov 2008 18:44:30 -0500 Masami Hiramatsu wrote:
    >
    >> Increase the size of kprobe hash table to 512. It's useful when hundreds
    >> of kprobes were used in the kernel because current size is just 64.
    >>

    >
    > "useful" is a bit vague. How big is the problem which this solves, and
    > how well did it solve it?


    For example, when probing enters and exits of syscall-related functions,
    we need more than 500 probes. In that case, each hlist would have 8
    elements in average. With this patch, the hlist would have 1 element in
    average.

    I agree that there may be many opinions about what is the best suited size.
    Why I chose 512 was that I thought the table (byte) size was less than or
    equal 4096 even on 64-bit arch.

    > See, someone (me) needs to decide whether to merge this and if so,
    > whether to merge it into 2.6.29, 2.6.28, 2.6.27.x, 2.6.26.x and
    > 2.6.25.x. I'll need more information to make that decision, but I do
    > not have it.


    I think this improves performance just a bit.
    So I think it would not be needed for 2.6.27.x or older kernel.

    Thank you,

    >
    >> --- 2.6.28-rc3.orig/kernel/kprobes.c
    >> +++ 2.6.28-rc3/kernel/kprobes.c
    >> @@ -49,7 +49,7 @@
    >> #include
    >> #include
    >>
    >> -#define KPROBE_HASH_BITS 6
    >> +#define KPROBE_HASH_BITS 9
    >> #define KPROBE_TABLE_SIZE (1 << KPROBE_HASH_BITS)

    >
    >


    --
    Masami Hiramatsu

    Software Engineer
    Hitachi Computer Products (America) Inc.
    Software Solutions Division

    e-mail: mhiramat@redhat.com

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

  4. Re: [PATCH] kprobe: increase kprobe_hash_table size

    On Fri, 07 Nov 2008 19:18:54 -0500 Masami Hiramatsu wrote:

    > Hi Andrew,
    >
    > Andrew Morton wrote:
    > > On Fri, 07 Nov 2008 18:44:30 -0500 Masami Hiramatsu wrote:
    > >
    > >> Increase the size of kprobe hash table to 512. It's useful when hundreds
    > >> of kprobes were used in the kernel because current size is just 64.
    > >>

    > >
    > > "useful" is a bit vague. How big is the problem which this solves, and
    > > how well did it solve it?

    >
    > For example, when probing enters and exits of syscall-related functions,
    > we need more than 500 probes. In that case, each hlist would have 8
    > elements in average. With this patch, the hlist would have 1 element in
    > average.
    >
    > I agree that there may be many opinions about what is the best suited size.
    > Why I chose 512 was that I thought the table (byte) size was less than or
    > equal 4096 even on 64-bit arch.


    Well...

    text data bss dec hex filename
    7036 744 9380 17160 4308 kernel/kprobes.o
    7048 744 73892 81684 13f14 kernel/kprobes.o

    That's 64 kbytes more memory. It will be kretprobe_table_locks[] which
    is hurting here, due to the ____cacheline_aligned.

    I expected CONFIG_X86_VSMP=y to make this far worse, but fortunately
    that only affects ____cacheline_internodealigned_in_smp.

    btw, that array wastes a ton of memory on uniprocessor builds. Using
    ____cacheline_aligned_in_smp should fix that.

    Please always check these thigns with /usr/bin/size.

    btw2, could/should kprobe_table[] and kretprobe_inst_table[] be
    aggregated into kretprobe_table_locks[]? That would save some memory
    and might save some cache misses as well?


    Anyway, enough pos-facto code review. Is this change which you're
    proposing worth increasing kernel memory usage by 64k?
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] kprobe: increase kprobe_hash_table size

    Andrew Morton wrote:
    >> I agree that there may be many opinions about what is the best suited size.
    >> Why I chose 512 was that I thought the table (byte) size was less than or
    >> equal 4096 even on 64-bit arch.

    >
    > Well...
    >
    > text data bss dec hex filename
    > 7036 744 9380 17160 4308 kernel/kprobes.o
    > 7048 744 73892 81684 13f14 kernel/kprobes.o
    >
    > That's 64 kbytes more memory. It will be kretprobe_table_locks[] which
    > is hurting here, due to the ____cacheline_aligned.


    Oops! It's really bad.


    > I expected CONFIG_X86_VSMP=y to make this far worse, but fortunately
    > that only affects ____cacheline_internodealigned_in_smp.
    >
    > btw, that array wastes a ton of memory on uniprocessor builds. Using
    > ____cacheline_aligned_in_smp should fix that.
    >
    > Please always check these thigns with /usr/bin/size.


    I see. I'll check that and try to find the best way...

    > btw2, could/should kprobe_table[] and kretprobe_inst_table[] be
    > aggregated into kretprobe_table_locks[]? That would save some memory
    > and might save some cache misses as well?


    Indeed, thank you for good idea.

    > Anyway, enough pos-facto code review. Is this change which you're
    > proposing worth increasing kernel memory usage by 64k?


    Not really. Hmm, I have to investigate more on this problem.

    Thanks a lot.

    --
    Masami Hiramatsu

    Software Engineer
    Hitachi Computer Products (America) Inc.
    Software Solutions Division

    e-mail: mhiramat@redhat.com

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

  6. Re: [PATCH] kprobe: increase kprobe_hash_table size

    On Fri, 07 Nov 2008 21:33:49 -0500 Masami Hiramatsu wrote:

    > Not really. Hmm, I have to investigate more on this problem.


    OK

    Meanwhile, how does this look?



    From: Andrew Morton

    We only need the cacheline padding on SMP kernels. Saves 6k:

    text data bss dec hex filename
    5713 388 2632 8733 221d kernel/kprobes.o
    5713 388 8840 14941 3a5d kernel/kprobes.o

    Cc: Masami Hiramatsu
    Cc: Ananth N Mavinakayanahalli
    Signed-off-by: Andrew Morton
    ---

    kernel/kprobes.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    diff -puN kernel/kprobes.c~a kernel/kprobes.c
    --- a/kernel/kprobes.c~a
    +++ a/kernel/kprobes.c
    @@ -72,7 +72,7 @@ static bool kprobe_enabled;
    DEFINE_MUTEX(kprobe_mutex); /* Protects kprobe_table */
    static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
    static struct {
    - spinlock_t lock ____cacheline_aligned;
    + spinlock_t lock ____cacheline_aligned_in_smp;
    } kretprobe_table_locks[KPROBE_TABLE_SIZE];

    static spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
    _

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

  7. Re: [PATCH] kprobe: increase kprobe_hash_table size

    Andrew Morton wrote:
    > On Fri, 07 Nov 2008 21:33:49 -0500 Masami Hiramatsu wrote:
    >
    >> Not really. Hmm, I have to investigate more on this problem.

    >
    > OK
    >
    > Meanwhile, how does this look?


    Great! That is enough acceptable.
    Thank you very much!


    > From: Andrew Morton
    >
    > We only need the cacheline padding on SMP kernels. Saves 6k:
    >
    > text data bss dec hex filename
    > 5713 388 2632 8733 221d kernel/kprobes.o
    > 5713 388 8840 14941 3a5d kernel/kprobes.o
    >
    > Cc: Masami Hiramatsu
    > Cc: Ananth N Mavinakayanahalli
    > Signed-off-by: Andrew Morton


    Acked-by: Masami Hiramatsu

    > ---
    >
    > kernel/kprobes.c | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff -puN kernel/kprobes.c~a kernel/kprobes.c
    > --- a/kernel/kprobes.c~a
    > +++ a/kernel/kprobes.c
    > @@ -72,7 +72,7 @@ static bool kprobe_enabled;
    > DEFINE_MUTEX(kprobe_mutex); /* Protects kprobe_table */
    > static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
    > static struct {
    > - spinlock_t lock ____cacheline_aligned;
    > + spinlock_t lock ____cacheline_aligned_in_smp;
    > } kretprobe_table_locks[KPROBE_TABLE_SIZE];
    >
    > static spinlock_t *kretprobe_table_lock_ptr(unsigned long hash)
    > _
    >


    --
    Masami Hiramatsu

    Software Engineer
    Hitachi Computer Products (America) Inc.
    Software Solutions Division

    e-mail: mhiramat@redhat.com

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

  8. Re: [PATCH] kprobe: increase kprobe_hash_table size

    Masami Hiramatsu wrote:
    > Andrew Morton wrote:
    >> On Fri, 07 Nov 2008 21:33:49 -0500 Masami Hiramatsu wrote:
    >>
    >>> Not really. Hmm, I have to investigate more on this problem.

    >> OK
    >>
    >> Meanwhile, how does this look?

    >
    > Great! That is enough acceptable.


    However, we still have to find reasonable way on SMP...

    Thank you,

    --
    Masami Hiramatsu

    Software Engineer
    Hitachi Computer Products (America) Inc.
    Software Solutions Division

    e-mail: mhiramat@redhat.com

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

+ Reply to Thread