Skip to contentSkip to conversation

scsi: sli4: Remove the buggy code

PATCH In review [email protected] 5 participants 5 messages Aug 17, 2023

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 scsi: sli4: Remove the buggy code

From: Rajeshwar R Shinde <[email protected]>

In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions evaluates the same expression and gives same output. Also the variable bls->local_n_port_id_dword is not used anywhere. Therefore removing the redundant code.

This fixes coccinelle warning such as:
drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible condition with no effect (if == else)

---
v1->v2
Modified patch and verified with checkpatch.pl.

---
 drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
 1 file changed, 7 deletions(-)
drivers/scsi/elx/libefc_sli/sli4.c +0 -7
index 8f96049f62dd..af661b769464 100644
@@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf,
 		SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
 		bls->context_tag = cpu_to_le16(params->vpi);
 
-		if (params->s_id != U32_MAX)
-			bls->local_n_port_id_dword |=
-				cpu_to_le32(params->s_id & 0x00ffffff);
-		else
-			bls->local_n_port_id_dword |=
-				cpu_to_le32(params->s_id & 0x00ffffff);
-
 		dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
 			       (params->d_id & SLI4_BLS_RSP_RID);
 
Signature · 6 lines
2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
#1 lore raw
Quoted 26 lines · On Thu, Aug 17, 2023 at 04:07:51PM +0530, [email protected] wrote:

From: Rajeshwar R Shinde <[email protected]>

In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions evaluates the same expression and gives same output. Also the variable bls->local_n_port_id_dword is not used anywhere. Therefore removing the redundant code.

This fixes coccinelle warning such as:
drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible condition with no effect (if == else)

---
v1->v2
Modified patch and verified with checkpatch.pl.

---
 drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
 1 file changed, 7 deletions(-)
drivers/scsi/elx/libefc_sli/sli4.c +0 -0
index 8f96049f62dd..af661b769464 100644

drivers/scsi/elx/libefc_sli/sli4.c sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf, · line 2326

Quoted 10 lines
drivers/scsi/elx/libefc_sli/sli4.c +0 -7
@@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf,
                SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
                bls->context_tag = cpu_to_le16(params->vpi);
 
-               if (params->s_id != U32_MAX)
-                       bls->local_n_port_id_dword |=
-                               cpu_to_le32(params->s_id & 0x00ffffff);
-               else
-                       bls->local_n_port_id_dword |=
-                               cpu_to_le32(params->s_id & 0x00ffffff);
-

omg, it is not an unused variable. Whole bls is a HW descriptor, all of its variables are used by HW. You should keep v1 version of the patch. According to the comment at the beginning of the funciton s_id here shall be != U32_MAX. That is an explanation for your v1 patch.

Quoted 6 lines
drivers/scsi/elx/libefc_sli/sli4.c +0 -1
                dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
                               (params->d_id & SLI4_BLS_RSP_RID);
 
--

2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

#2 lore raw
Quoted 40 lines · Dmitry Bogdanov
Quoted 17 lines · On Thu, Aug 17, 2023 at 04:07:51PM +0530, [email protected] wrote:

From: Rajeshwar R Shinde <[email protected]>

In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions evaluates the same expression and gives same output. Also the variable bls->local_n_port_id_dword is not used anywhere. Therefore removing the redundant code.

This fixes coccinelle warning such as:
drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible condition with no effect (if == else)

---
v1->v2
Modified patch and verified with checkpatch.pl.

Review

Quoted 5 lines
---
 drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
 1 file changed, 7 deletions(-)
(quoted diff) +0 -0

b/drivers/scsi/elx/libefc_sli/sli4.c

drivers/scsi/elx/libefc_sli/sli4.c sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void

Quoted 4 lines

index 8f96049f62dd..af661b769464 100644

drivers/scsi/elx/libefc_sli/sli4.c +0 -0
@@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void

*buf,

drivers/scsi/elx/libefc_sli/sli4.c sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf, · line 2326

Quoted 10 lines
drivers/scsi/elx/libefc_sli/sli4.c +0 -7
                SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
                bls->context_tag = cpu_to_le16(params->vpi);
 
-               if (params->s_id != U32_MAX)
-                       bls->local_n_port_id_dword |=
-                               cpu_to_le32(params->s_id & 0x00ffffff);
-               else
-                       bls->local_n_port_id_dword |=
-                               cpu_to_le32(params->s_id & 0x00ffffff);
-
Quoted 11 lines

omg, it is not an unused variable. Whole bls is a HW descriptor, all of its variables are used by HW. You should keep v1 version of the patch. According to the comment at the beginning of the funciton s_id here shall be != U32_MAX. That is an explanation for your v1 patch.

Quoted 6 lines
drivers/scsi/elx/libefc_sli/sli4.c +0 -1
                dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
                               (params->d_id & SLI4_BLS_RSP_RID);
 
--

2.25.1

local_n_port_id_dword is not an unused variable. As Dimtry pointed out all the above variables are used by the HW.

Signature · 11 lines
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

#3 lore raw
Quoted 12 lines · Ram Kishore Vegesna via Linux-kernel-mentees
Signature · 11 lines
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Now deleted.

(hint, this footer is not compatible with kernel development, please fix your email system...)
_______________________________________________
Linux-kernel-mentees mailing list [email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

#4 lore raw
In reply to Dmitry Bogdanov
Quoted 30 lines · Dmitry Bogdanov
Quoted 29 lines · On Thu, Aug 17, 2023 at 04:07:51PM +0530, [email protected] wrote:

From: Rajeshwar R Shinde <[email protected]>

In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions evaluates the same expression and gives same output. Also the variable bls->local_n_port_id_dword is not used anywhere. Therefore removing the redundant code.

This fixes coccinelle warning such as:
drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible condition with no effect (if == else)

---
v1->v2
Modified patch and verified with checkpatch.pl.

---
 drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
 1 file changed, 7 deletions(-)
drivers/scsi/elx/libefc_sli/sli4.c +0 -0
index 8f96049f62dd..af661b769464 100644
@@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf,
                SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
                bls->context_tag = cpu_to_le16(params->vpi);
 
Quoted 13 lines

drivers/scsi/elx/libefc_sli/sli4.c sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf, · line 2326

Quoted 7 lines
drivers/scsi/elx/libefc_sli/sli4.c +0 -7
-               if (params->s_id != U32_MAX)
-                       bls->local_n_port_id_dword |=
-                               cpu_to_le32(params->s_id & 0x00ffffff);
-               else
-                       bls->local_n_port_id_dword |=
-                               cpu_to_le32(params->s_id & 0x00ffffff);
-

omg, it is not an unused variable. Whole bls is a HW descriptor, all of its variables are used by HW. You should keep v1 version of the patch. According to the comment at the beginning of the funciton s_id here shall be != U32_MAX. That is an explanation for your v1 patch.

Thanks for your comments.

Quoted 6 lines
Quoted 6 lines
drivers/scsi/elx/libefc_sli/sli4.c +0 -1
                dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
                               (params->d_id & SLI4_BLS_RSP_RID);
 
--

2.25.1

Signature · 5 lines
~Randy
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees