lkml.org 
[lkml]   [2014]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure
On 07/04/2014 03:37 PM, Julia Lawall wrote:
>
>
> On Fri, 4 Jul 2014, Elliott, Robert (Server Storage) wrote:
>
>>
>>
>>> -----Original Message-----
>>> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
>>> owner@vger.kernel.org] On Behalf Of Himangi Saraogi
>>> Sent: Friday, 04 July, 2014 1:28 PM
>>> To: Vikas Chaudhary; iscsi-driver@qlogic.com; James E.J. Bottomley; linux-
>>> scsi@vger.kernel.org; linux-kernel@vger.kernel.org
>>> Cc: julia.lawall@lip6.fr
>>> Subject: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure
>>>
>>> In this code, 0 is returned on memory allocation failure, even though
>>> other failures return -ENOMEM or other similar values.
>>>
>>> A simplified version of the Coccinelle semantic match that finds this
>>> problem is as follows:
>>>
>>> // <smpl>
>>> @@
>>> expression ret;
>>> expression x,e1,e2,e3;
>>> identifier alloc;
>>> @@
>>>
>>> ret = 0
>>> ... when != ret = e1
>>> *x = alloc(...)
>>> ... when != ret = e2
>>> if (x == NULL) { ... when != ret = e3
>>> return ret;
>>> }
>>> // </smpl>
>>>
>>> Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
>>> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>>> ---
>>> drivers/scsi/qla4xxx/ql4_os.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
>>> index c5d9564..72ba671 100644
>>> --- a/drivers/scsi/qla4xxx/ql4_os.c
>>> +++ b/drivers/scsi/qla4xxx/ql4_os.c
>>> @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host
>>> *shost, char *buf, int len)
>>> if (!ql_iscsi_stats) {
>>> ql4_printk(KERN_ERR, ha,
>>> "Unable to allocate memory for iscsi stats\n");
>>> + ret = -ENOMEM;
>>> goto exit_host_stats;
>>> }
>>>
>>
>> Also, the only caller of this function doesn't use the return
>> value - it's overwritten by another function call:
>>
>> drivers/scsi/scsi_transport_iscsi.c:
>> err = transport->get_host_stats(shost, buf, host_stats_size);
>>
>> actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size);
>> skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));
>> nlhhost_stats->nlmsg_len = actual_size;
>>
>> err = iscsi_multicast_skb(skbhost_stats, ISCSI_NL_GRP_ISCSID,
>> GFP_KERNEL);
>
> Maybe that is intentional?

I think it was a mistake. There is also one more issue in
qla4xxx_get_host_stats where it is returning an internal qlogic
specific error value. This patch should fix all issues.


[PATCH] qla4xxx/iscsi: fix get_host_stats error propagation

This patch fixes 2 bugs.

1. qla4xxx was not always returning -EXYZ error codes when
qla4xxx_get_host_stats failed.

2. iscsi_get_host_stats was dropping the error code returned
by drivers like qla4xxx.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 459b9f7..e4dc3e0 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host *shost, char *buf, int len)
if (!ql_iscsi_stats) {
ql4_printk(KERN_ERR, ha,
"Unable to allocate memory for iscsi stats\n");
+ ret = -ENOMEM;
goto exit_host_stats;
}

@@ -1058,6 +1059,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host *shost, char *buf, int len)
if (ret != QLA_SUCCESS) {
ql4_printk(KERN_ERR, ha,
"Unable to retrieve iscsi stats\n");
+ ret = -EIO;
goto exit_host_stats;
}
host_stats->mactx_frames = le64_to_cpu(ql_iscsi_stats->mac_tx_frames);
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 0102a2d..ea2996a 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3467,6 +3467,10 @@ iscsi_get_host_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
memset(buf, 0, host_stats_size);

err = transport->get_host_stats(shost, buf, host_stats_size);
+ if (err) {
+ kfree(skbhost_stats);
+ goto exit_host_stats;
+ }

actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size);
skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));


\
 
 \ /
  Last update: 2014-07-07 19:22    [W:0.046 / U:0.696 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site