2012-02-05 22:27:31

by Willy Tarreau

[permalink] [raw]
Subject: [PATCH 32/91] libsas: remove expander from dev list on error

2.6.27-longterm review patch. If anyone has any objections, please let us know.

------------------

commit 5911e963d3718e306bcac387b83e259aa4228896 upstream.

If expander discovery fails (sas_discover_expander()), remove the
expander from the port device list (sas_ex_discover_expander()),
before freeing it. Else the list is corrupted and, e.g., when we
attempt to send SMP commands to other devices, the kernel oopses.

Signed-off-by: Luben Tuikov <[email protected]>
Reviewed-by: Jack Wang <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/scsi/libsas/sas_expander.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

Index: longterm-2.6.27/drivers/scsi/libsas/sas_expander.c
===================================================================
--- longterm-2.6.27.orig/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:34.059914940 +0100
+++ longterm-2.6.27/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:39.404915902 +0100
@@ -839,6 +839,9 @@

res = sas_discover_expander(child);
if (res) {
+ spin_lock_irq(&parent->port->dev_list_lock);
+ list_del(&child->dev_list_node);
+ spin_unlock_irq(&parent->port->dev_list_lock);
kfree(child);
return NULL;
}


2012-02-05 23:54:41

by Luben Tuikov

[permalink] [raw]
Subject: Re: [PATCH 32/91] libsas: remove expander from dev list on error

Isn't this my patch? Are you submitting it as your own?

Luben


On Feb 5, 2012, at 14:10, Willy Tarreau <[email protected]> wrote:

> 2.6.27-longterm review patch. If anyone has any objections, please let us know.
>
> ------------------
>
> commit 5911e963d3718e306bcac387b83e259aa4228896 upstream.
>
> If expander discovery fails (sas_discover_expander()), remove the
> expander from the port device list (sas_ex_discover_expander()),
> before freeing it. Else the list is corrupted and, e.g., when we
> attempt to send SMP commands to other devices, the kernel oopses.
>
> Signed-off-by: Luben Tuikov <[email protected]>
> Reviewed-by: Jack Wang <[email protected]>
> Signed-off-by: James Bottomley <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/scsi/libsas/sas_expander.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> Index: longterm-2.6.27/drivers/scsi/libsas/sas_expander.c
> ===================================================================
> --- longterm-2.6.27.orig/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:34.059914940 +0100
> +++ longterm-2.6.27/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:39.404915902 +0100
> @@ -839,6 +839,9 @@
>
> res = sas_discover_expander(child);
> if (res) {
> + spin_lock_irq(&parent->port->dev_list_lock);
> + list_del(&child->dev_list_node);
> + spin_unlock_irq(&parent->port->dev_list_lock);
> kfree(child);
> return NULL;
> }
>
>

2012-02-06 00:53:21

by Wanlong Gao

[permalink] [raw]
Subject: Re: [PATCH 32/91] libsas: remove expander from dev list on error

On 02/06/2012 07:48 AM, Luben Tuikov wrote:

> Isn't this my patch? Are you submitting it as your own?


No, he just wanna backport to the stable.

>
> Luben
>
>
> On Feb 5, 2012, at 14:10, Willy Tarreau <[email protected]> wrote:
>
>> 2.6.27-longterm review patch. If anyone has any objections, please let us know.
>>
>> ------------------
>>
>> commit 5911e963d3718e306bcac387b83e259aa4228896 upstream.
>>
>> If expander discovery fails (sas_discover_expander()), remove the
>> expander from the port device list (sas_ex_discover_expander()),
>> before freeing it. Else the list is corrupted and, e.g., when we
>> attempt to send SMP commands to other devices, the kernel oopses.
>>
>> Signed-off-by: Luben Tuikov <[email protected]>
>> Reviewed-by: Jack Wang <[email protected]>
>> Signed-off-by: James Bottomley <[email protected]>
>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>> ---
>> drivers/scsi/libsas/sas_expander.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> Index: longterm-2.6.27/drivers/scsi/libsas/sas_expander.c
>> ===================================================================
>> --- longterm-2.6.27.orig/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:34.059914940 +0100
>> +++ longterm-2.6.27/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:39.404915902 +0100
>> @@ -839,6 +839,9 @@
>>
>> res = sas_discover_expander(child);
>> if (res) {
>> + spin_lock_irq(&parent->port->dev_list_lock);
>> + list_del(&child->dev_list_node);
>> + spin_unlock_irq(&parent->port->dev_list_lock);
>> kfree(child);
>> return NULL;
>> }
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2012-02-06 01:20:19

by Luben Tuikov

[permalink] [raw]
Subject: Re: [PATCH 32/91] libsas: remove expander from dev list on error

Where is the "From:" tag that would appear in "git log"?

Luben


On Feb 5, 2012, at 16:52, Wanlong Gao <[email protected]> wrote:

> On 02/06/2012 07:48 AM, Luben Tuikov wrote:
>
>> Isn't this my patch? Are you submitting it as your own?
>
>
> No, he just wanna backport to the stable.
>
>>
>> Luben
>>
>>
>> On Feb 5, 2012, at 14:10, Willy Tarreau <[email protected]> wrote:
>>
>>> 2.6.27-longterm review patch. If anyone has any objections, please let us know.
>>>
>>> ------------------
>>>
>>> commit 5911e963d3718e306bcac387b83e259aa4228896 upstream.
>>>
>>> If expander discovery fails (sas_discover_expander()), remove the
>>> expander from the port device list (sas_ex_discover_expander()),
>>> before freeing it. Else the list is corrupted and, e.g., when we
>>> attempt to send SMP commands to other devices, the kernel oopses.
>>>
>>> Signed-off-by: Luben Tuikov <[email protected]>
>>> Reviewed-by: Jack Wang <[email protected]>
>>> Signed-off-by: James Bottomley <[email protected]>
>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>> ---
>>> drivers/scsi/libsas/sas_expander.c | 3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> Index: longterm-2.6.27/drivers/scsi/libsas/sas_expander.c
>>> ===================================================================
>>> --- longterm-2.6.27.orig/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:34.059914940 +0100
>>> +++ longterm-2.6.27/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:39.404915902 +0100
>>> @@ -839,6 +839,9 @@
>>>
>>> res = sas_discover_expander(child);
>>> if (res) {
>>> + spin_lock_irq(&parent->port->dev_list_lock);
>>> + list_del(&child->dev_list_node);
>>> + spin_unlock_irq(&parent->port->dev_list_lock);
>>> kfree(child);
>>> return NULL;
>>> }
>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
>

2012-02-06 06:26:43

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 32/91] libsas: remove expander from dev list on error

On Sun, Feb 05, 2012 at 05:14:26PM -0800, Luben Tuikov wrote:
> Where is the "From:" tag that would appear in "git log"?

Here's the complete patch scheduled for merging :

From da229078845ada4d7b0b49a020c8eaf49420cec9 Mon Sep 17 00:00:00 2001
From: Luben Tuikov <[email protected]>
Date: Tue, 26 Jul 2011 23:10:48 -0700
Subject: libsas: remove expander from dev list on error

commit 5911e963d3718e306bcac387b83e259aa4228896 upstream.

If expander discovery fails (sas_discover_expander()), remove the
expander from the port device list (sas_ex_discover_expander()),
before freeing it. Else the list is corrupted and, e.g., when we
attempt to send SMP commands to other devices, the kernel oopses.

Signed-off-by: Luben Tuikov <[email protected]>
Reviewed-by: Jack Wang <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/scsi/libsas/sas_expander.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

Index: longterm-2.6.27/drivers/scsi/libsas/sas_expander.c
===================================================================
--- longterm-2.6.27.orig/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:34.059914940 +0100
+++ longterm-2.6.27/drivers/scsi/libsas/sas_expander.c 2012-02-05 22:34:39.404915902 +0100
@@ -839,6 +839,9 @@

res = sas_discover_expander(child);
if (res) {
+ spin_lock_irq(&parent->port->dev_list_lock);
+ list_del(&child->dev_list_node);
+ spin_unlock_irq(&parent->port->dev_list_lock);
kfree(child);
return NULL;
}

You're clearly the one who's credited for the patch, both by the From: and
Signed-off-by tags. I am always very careful about credit attributions, and
that's even what scares me when I have to force to apply patches by hand.
I think you were surprized not to see you in the From just because of the
way the review scripts assembles the patches in mails. I'm using quilt this
way to build the mails :

quilt mail \
-m "$(sed -e "s/XX\.YY/$SUBLEVEL.$REV/g" .quilt/header;git diff --stat)" \
--from "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" \
--subject "$VERSION.$PATCHLEVEL.$SUBLEVEL.$REV-longterm review" \
--prefix "PATCH" \
--to "[email protected], [email protected]" \
--mbox mbox

I don't think there's anything wrong with this. I you have suggestions to
improve the output readability, feel free to suggest so.

Best regards,
Willy