mpt fusion: fix for previous patch taskmgmt in progress should be cleared before return

PATCH In review Kashyap, Desai 2 participants 5 messages Apr 3, 2009 – Apr 6, 2009

Subsystems (2)

SCSI LSI Logic MPT fusion drivers (FC/SAS/SPI)

lore

Share & tools

lore

Kashyap, Desai

author

2009-04-03 12:08 UTC #0 lore raw

PATCH 1/6 mpt fusion: fix for previous patch taskmgmt in progress should be cleared before return

Small fix for previous Patch applied on 26/02/2009: subject line : "mpt fusion: config, Taskmgmt cmd, Internal cmd, Ioctl cmd changes".

Before returning from function make sure that taskmgmt in progress bit is cleared.
---

Signed-off-by Kashyap Desai <[email protected]>

---

drivers/message/fusion/mptsas.c +17 -6

index 842e4f8..4e90870 100644
@@ -553,18 +553,20 @@ mptsas_target_reset(MPT_ADAPTER *ioc, u8 channel, u8 id)
 {
 	MPT_FRAME_HDR	*mf;
 	SCSITaskMgmt_t	*pScsiTm;
+
 	if (mpt_set_taskmgmt_in_progress_flag(ioc) != 0)
 		return 0;
 
-
-	mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc);
-	if (mf == NULL) {
+	if ((mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc)) == NULL) {
 		dfailprintk(ioc, printk(MYIOC_s_WARN_FMT
-			"%s, no msg frames @%d!!\n",
-			ioc->name, __func__, __LINE__));
-		return 0;
+			"%s, no msg frames @%d!!\n", ioc->name,
+			__func__, __LINE__));
+		goto out_fail;
 	}
 
+	dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT "TaskMgmt request (mf=%p)\n",
+		ioc->name, mf));
+
 	/* Format the Request
 	 */
 	pScsiTm = (SCSITaskMgmt_t *) mf;

@@ -577,9 +579,18 @@ mptsas_target_reset(MPT_ADAPTER *ioc, u8 channel, u8 id)
 
 	DBG_DUMP_TM_REQUEST_FRAME(ioc, (u32 *)mf);
 
+	dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT
+	   "TaskMgmt type=%d (sas device delete) fw_channel = %d fw_id = %d)\n",
+	   ioc->name, MPI_SCSITASKMGMT_TASKTYPE_TARGET_RESET, channel, id));
+
 	mpt_put_msg_frame_hi_pri(mptsasDeviceResetCtx, ioc, mf);
 
 	return 1;
+
+ out_fail:
+
+	mpt_clear_taskmgmt_in_progress_flag(ioc);
+	return 0;
 }
 
 /**

James Bottomley

maintainer

2009-04-03 17:38 UTC #1 lore raw

drivers/message/fusion/mptsas.c mptsas_target_reset(MPT_ADAPTER *ioc, u8 channel, u8 id) · line 561

Quoted 5 lines · Desai

drivers/message/fusion/mptsas.c +1 -3

-
-       mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc);
-       if (mf == NULL) {
+       if ((mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc)) == NULL) {
                dfailprintk(ioc, printk(MYIOC_s_WARN_FMT

This is making a clear statement (assignment then check) less clear for no practical purpose that I can see.

James

Desai, Kashyap

author

2009-04-04 09:45 UTC #2 lore raw

James,

After creating patch, I usually run check_patch script available in scripts directory of linux source tree. Below changes are just to remove warning and error related to coding standards from generated patch.

If you suggest, not to have those changes merge with actual logical patch then please let me know. I can rework and resubmit.

Thanks
Kashyap
________________________________________
From: James Bottomley [[email protected]]
Sent: Friday, April 03, 2009 1:38 PM
To: Desai, Kashyap
Cc: [email protected]; Moore, Eric; Prakash, Sathya
Subject: Re: [PATCH 1/6] mpt fusion: fix for previous patch taskmgmt in progress should be cleared before return

drivers/message/fusion/mptsas.c mptsas_target_reset(MPT_ADAPTER *ioc, u8 channel, u8 id) · line 561

Quoted 5 lines · Desai

drivers/message/fusion/mptsas.c +1 -3

-
-       mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc);
-       if (mf == NULL) {
+       if ((mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc)) == NULL) {
                dfailprintk(ioc, printk(MYIOC_s_WARN_FMT

This is making a clear statement (assignment then check) less clear for no practical purpose that I can see.

James

James Bottomley

maintainer

2009-04-04 13:17 UTC #3 lore raw

Quoted 4 lines · Kashyap

James,
After creating patch, I usually run check_patch script available in scripts directory of linux source tree. Below changes are just to remove warning and error related to coding standards from generated patch.

Hmm, then we have an error in checkpatch.pl if it's saying that

x = y
if (x == NULL)

should become

if ((x = y) == NULL)

Can you attach the warning so I can get the checkpatch maintainers to explain what's going on?

Thanks,

James

Quoted 30 lines

If you suggest, not to have those changes merge with actual logical patch then please let me know. I can rework and resubmit.
Thanks
Kashyap
________________________________________
From: James Bottomley [[email protected]]
Sent: Friday, April 03, 2009 1:38 PM
To: Desai, Kashyap
Cc: [email protected]; Moore, Eric; Prakash, Sathya
Subject: Re: [PATCH 1/6] mpt fusion: fix for previous patch taskmgmt in progress should be cleared before return
drivers/message/fusion/mptsas.c mptsas_target_reset(MPT_ADAPTER *ioc, u8 channel, u8 id) · line 561
Quoted 5 lines · Desai
drivers/message/fusion/mptsas.c +1 -3
-
-       mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc);
-       if (mf == NULL) {
+       if ((mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc)) == NULL) {
                dfailprintk(ioc, printk(MYIOC_s_WARN_FMT
This is making a clear statement (assignment then check) less clear for no practical purpose that I can see.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Desai, Kashyap

author

2009-04-06 04:53 UTC #4 lore raw

James,

I am really sorry for my previous reply. Actually checkpatch is reporting correctly, but my understanding was wrong. I am going to correct it and resending it back.

Thanks
Kashyap

-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Saturday, April 04, 2009 6:47 PM
To: Desai, Kashyap
Cc: [email protected]; Moore, Eric; Prakash, Sathya
Subject: RE: [PATCH 1/6] mpt fusion: fix for previous patch taskmgmt in progress should be cleared before return

Quoted 4 lines · Kashyap

James,
After creating patch, I usually run check_patch script available in scripts directory of linux source tree. Below changes are just to remove warning and error related to coding standards from generated patch.

Hmm, then we have an error in checkpatch.pl if it's saying that

x = y
if (x == NULL)

should become

if ((x = y) == NULL)

Can you attach the warning so I can get the checkpatch maintainers to explain what's going on?

Thanks,

James

Quoted 30 lines

If you suggest, not to have those changes merge with actual logical patch then please let me know. I can rework and resubmit.
Thanks
Kashyap
________________________________________
From: James Bottomley [[email protected]]
Sent: Friday, April 03, 2009 1:38 PM
To: Desai, Kashyap
Cc: [email protected]; Moore, Eric; Prakash, Sathya
Subject: Re: [PATCH 1/6] mpt fusion: fix for previous patch taskmgmt in progress should be cleared before return
drivers/message/fusion/mptsas.c mptsas_target_reset(MPT_ADAPTER *ioc, u8 channel, u8 id) · line 561
Quoted 5 lines · Desai
drivers/message/fusion/mptsas.c +1 -3
-
-       mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc);
-       if (mf == NULL) {
+       if ((mf = mpt_get_msg_frame(mptsasDeviceResetCtx, ioc)) == NULL) {
                dfailprintk(ioc, printk(MYIOC_s_WARN_FMT
This is making a clear statement (assignment then check) less clear for no practical purpose that I can see.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html