[PATCH] 2.6.25+: Fix cpu hotplug in softirq code - Kernel

This is a discussion on [PATCH] 2.6.25+: Fix cpu hotplug in softirq code - Kernel ; Hello Olof, currently cpu hotplug (unplug) seems broken on s390 and likely others. On cpu unplug the system starts to behave very strange and hangs. I bisected the problem to the following commit: ----- commit 48f20a9a9488c432fc86df1ff4b7f4fa895d1183 Author: Olof Johansson Date: ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: [PATCH] 2.6.25+: Fix cpu hotplug in softirq code

  1. [PATCH] 2.6.25+: Fix cpu hotplug in softirq code

    Hello Olof,

    currently cpu hotplug (unplug) seems broken on s390 and likely others. On cpu
    unplug the system starts to behave very strange and hangs.

    I bisected the problem to the following commit:

    -----
    commit 48f20a9a9488c432fc86df1ff4b7f4fa895d1183
    Author: Olof Johansson
    Date: Tue Mar 4 15:23:25 2008 -0800
    tasklets: execute tasklets in the same order they were queued
    -----

    Reverting this patch seems to fix the problem. I looked into takeover_tasklet
    and it seems that there is a way to corrupt the tail pointer of the current
    cpu. If the tasklet list of the frozen cpu is empty, the tail pointer of the
    current cpu points to the address of the head pointer of the stopped cpu and
    not to the next pointer of a tasklet_struct.

    This patch avoids the list splice of the list is empty and cpu hotplug seems
    to work as the tail pointer is not corrupted.
    Olof, can you look into that patch and ACK/NACK it so Andrew can push this to
    Linus, if appropriate?
    Please note that some lines are longer than 80 chars, but line-wrapping looked
    worse that this version.

    Signed-off-by: Christian Borntraeger

    ---
    kernel/softirq.c | 20 ++++++++++++--------
    1 file changed, 12 insertions(+), 8 deletions(-)

    Index: kvm/kernel/softirq.c
    ================================================== =================
    --- kvm.orig/kernel/softirq.c
    +++ kvm/kernel/softirq.c
    @@ -589,16 +589,20 @@ static void takeover_tasklets(unsigned i
    local_irq_disable();

    /* Find end, append list for that CPU. */
    - *__get_cpu_var(tasklet_vec).tail = per_cpu(tasklet_vec, cpu).head;
    - __get_cpu_var(tasklet_vec).tail = per_cpu(tasklet_vec, cpu).tail;
    - per_cpu(tasklet_vec, cpu).head = NULL;
    - per_cpu(tasklet_vec, cpu).tail = &per_cpu(tasklet_vec, cpu).head;
    + if (&per_cpu(tasklet_vec, cpu).head != per_cpu(tasklet_vec, cpu).tail) {
    + *(__get_cpu_var(tasklet_vec).tail) = per_cpu(tasklet_vec, cpu).head;
    + __get_cpu_var(tasklet_vec).tail = per_cpu(tasklet_vec, cpu).tail;
    + per_cpu(tasklet_vec, cpu).head = NULL;
    + per_cpu(tasklet_vec, cpu).tail = &per_cpu(tasklet_vec, cpu).head;
    + }
    raise_softirq_irqoff(TASKLET_SOFTIRQ);

    - *__get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).head;
    - __get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).tail;
    - per_cpu(tasklet_hi_vec, cpu).head = NULL;
    - per_cpu(tasklet_hi_vec, cpu).tail = &per_cpu(tasklet_hi_vec, cpu).head;
    + if (&per_cpu(tasklet_hi_vec, cpu).head != per_cpu(tasklet_hi_vec, cpu).tail) {
    + *__get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).head;
    + __get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).tail;
    + per_cpu(tasklet_hi_vec, cpu).head = NULL;
    + per_cpu(tasklet_hi_vec, cpu).tail = &per_cpu(tasklet_hi_vec, cpu).head;
    + }
    raise_softirq_irqoff(HI_SOFTIRQ);

    local_irq_enable();
    --
    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. [PATCH/resend] 2.6.25+: Fix cpu hotplug problem in softirq code

    This is a resend of a patch, which fixes a bug in cpu hotplug introduced
    after 2.6.25. Andrew, Olof, any opinions on this patch?

    Christian


    --- old mail ---

    Hello Olof,

    currently cpu hotplug (unplug) seems broken on s390 and likely others. On cpu
    unplug the system starts to behave very strange and hangs.

    I bisected the problem to the following commit:

    -----
    commit 48f20a9a9488c432fc86df1ff4b7f4fa895d1183
    Author: Olof Johansson
    Date: * Tue Mar 4 15:23:25 2008 -0800
    * * tasklets: execute tasklets in the same order they were queued
    -----

    Reverting this patch seems to fix the problem. I looked into takeover_tasklet
    and it seems that there is a way to corrupt the tail pointer of the current
    cpu. If the tasklet list of the frozen cpu is empty, the tail pointer of the
    current cpu points to the address of the head pointer of the stopped cpu and
    not to the next pointer of a tasklet_struct.

    This patch avoids the list splice of the list is empty and cpu hotplug seems
    to work as the tail pointer is not corrupted.
    Olof, can you look into that patch and ACK/NACK it so Andrew can push this to
    Linus, if appropriate?
    Please note that some lines are longer than 80 chars, but line-wrapping looked
    worse that this version.

    Signed-off-by: Christian Borntraeger
    ---
    kernel/softirq.c | 20 ++++++++++++--------
    1 file changed, 12 insertions(+), 8 deletions(-)

    Index: kvm/kernel/softirq.c
    ================================================== =================
    --- kvm.orig/kernel/softirq.c
    +++ kvm/kernel/softirq.c
    @@ -589,16 +589,20 @@ static void takeover_tasklets(unsigned i
    local_irq_disable();

    /* Find end, append list for that CPU. */
    - *__get_cpu_var(tasklet_vec).tail = per_cpu(tasklet_vec, cpu).head;
    - __get_cpu_var(tasklet_vec).tail = per_cpu(tasklet_vec, cpu).tail;
    - per_cpu(tasklet_vec, cpu).head = NULL;
    - per_cpu(tasklet_vec, cpu).tail = &per_cpu(tasklet_vec, cpu).head;
    + if (&per_cpu(tasklet_vec, cpu).head != per_cpu(tasklet_vec, cpu).tail) {
    + *(__get_cpu_var(tasklet_vec).tail) = per_cpu(tasklet_vec, cpu).head;
    + __get_cpu_var(tasklet_vec).tail = per_cpu(tasklet_vec, cpu).tail;
    + per_cpu(tasklet_vec, cpu).head = NULL;
    + per_cpu(tasklet_vec, cpu).tail = &per_cpu(tasklet_vec, cpu).head;
    + }
    raise_softirq_irqoff(TASKLET_SOFTIRQ);

    - *__get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).head;
    - __get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).tail;
    - per_cpu(tasklet_hi_vec, cpu).head = NULL;
    - per_cpu(tasklet_hi_vec, cpu).tail = &per_cpu(tasklet_hi_vec, cpu).head;
    + if (&per_cpu(tasklet_hi_vec, cpu).head != per_cpu(tasklet_hi_vec, cpu).tail) {
    + *__get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).head;
    + __get_cpu_var(tasklet_hi_vec).tail = per_cpu(tasklet_hi_vec, cpu).tail;
    + per_cpu(tasklet_hi_vec, cpu).head = NULL;
    + per_cpu(tasklet_hi_vec, cpu).tail = &per_cpu(tasklet_hi_vec, cpu).head;
    + }
    raise_softirq_irqoff(HI_SOFTIRQ);

    local_irq_enable();
    --
    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/resend] 2.6.25+: Fix cpu hotplug problem in softirq code


    * Christian Borntraeger wrote:

    > This is a resend of a patch, which fixes a bug in cpu hotplug
    > introduced after 2.6.25. Andrew, Olof, any opinions on this patch?


    hm, nice catch.

    Acked-by: Ingo Molnar

    Ingo
    --
    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] 2.6.25+: Fix cpu hotplug in softirq code

    On Thu, Apr 24, 2008 at 09:13:11PM +0200, Christian Borntraeger wrote:
    > Hello Olof,
    >
    > currently cpu hotplug (unplug) seems broken on s390 and likely others. On cpu
    > unplug the system starts to behave very strange and hangs.
    >
    > I bisected the problem to the following commit:
    >
    > -----
    > commit 48f20a9a9488c432fc86df1ff4b7f4fa895d1183
    > Author: Olof Johansson
    > Date: Tue Mar 4 15:23:25 2008 -0800
    > tasklets: execute tasklets in the same order they were queued
    > -----
    >
    > Reverting this patch seems to fix the problem. I looked into takeover_tasklet
    > and it seems that there is a way to corrupt the tail pointer of the current
    > cpu. If the tasklet list of the frozen cpu is empty, the tail pointer of the
    > current cpu points to the address of the head pointer of the stopped cpu and
    > not to the next pointer of a tasklet_struct.
    >
    > This patch avoids the list splice of the list is empty and cpu hotplug seems
    > to work as the tail pointer is not corrupted.
    > Olof, can you look into that patch and ACK/NACK it so Andrew can push this to
    > Linus, if appropriate?
    > Please note that some lines are longer than 80 chars, but line-wrapping looked
    > worse that this version.
    >
    > Signed-off-by: Christian Borntraeger


    I don't have a hotplug-capable system to test on, but the patch looks
    good to me. Good catch.

    Acked-by: Olof Johansson


    -Olof
    --
    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