RE: [PATCH 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
Hi, Xi About patch 3, I check the ffz code and found it will check ~0 conditions. Can you point me to the ~0 check in ffz code? I also feel like using __ffs64 makes the code simpler. Yes, it seem to be ok Does patch 1 look good to you? Thanks. Yes Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
On 11/9/12 2:30 AM, Xiangliang Yu wrote: Agree with James, and just need to do NOT operation one time Thanks for reviewing the patches. Okay I'll remove patch 2 in v2 then. About patch 3, I check the ffz code and found it will check ~0 conditions. Can you point me to the ~0 check in ffz code? I also feel like using __ffs64 makes the code simpler. Does patch 1 look good to you? Thanks. - xi -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
On 11/6/12 7:06 AM, James Bottomley wrote: Why is this necessary? As I read the reg set assignment code, it finds a free bit in the 64 bit register and uses that ... which can never be greater than 64 so there's no need for the check. This patch just tries to be more defensive for bit(reg_set) with a broken reg_set value. I agree with you that it's not that necessary. Agree with James, and just need to do NOT operation one time The other two look OK (probably redone as a single patch with a stable tag), but I'd like the input of the mvs people since it seems with the current code, we only use 32 bit regsets and probably hang if we go over that. The bug fix is either to enable the full 64 if it works, or possibly cap at 32 ... what works with all released devices? Thanks for reviewing. Yeah we'd better to wait for the input from the mvs people. About patch 3, I check the ffz code and found it will check ~0 conditions. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
On Mon, 2012-11-05 at 14:53 -0500, Xi Wang wrote: Invoking bit(n) with n = 64 is undefined behavior, since bit(n) does a 64-bit shift. This patch adds a check on the shifting amount. Why is this necessary? As I read the reg set assignment code, it finds a free bit in the 64 bit register and uses that ... which can never be greater than 64 so there's no need for the check. The other two look OK (probably redone as a single patch with a stable tag), but I'd like the input of the mvs people since it seems with the current code, we only use 32 bit regsets and probably hang if we go over that. The bug fix is either to enable the full 64 if it works, or possibly cap at 32 ... what works with all released devices? James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
On 11/6/12 7:06 AM, James Bottomley wrote: Why is this necessary? As I read the reg set assignment code, it finds a free bit in the 64 bit register and uses that ... which can never be greater than 64 so there's no need for the check. This patch just tries to be more defensive for bit(reg_set) with a broken reg_set value. I agree with you that it's not that necessary. The other two look OK (probably redone as a single patch with a stable tag), but I'd like the input of the mvs people since it seems with the current code, we only use 32 bit regsets and probably hang if we go over that. The bug fix is either to enable the full 64 if it works, or possibly cap at 32 ... what works with all released devices? Thanks for reviewing. Yeah we'd better to wait for the input from the mvs people. - xi -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] [SCSI] mvsas: fix shift in mvs_94xx_free_reg_set()
Invoking bit(n) with n = 64 is undefined behavior, since bit(n) does
a 64-bit shift. This patch adds a check on the shifting amount.
Signed-off-by: Xi Wang xi.w...@gmail.com
---
drivers/scsi/mvsas/mv_94xx.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/mvsas/mv_94xx.c b/drivers/scsi/mvsas/mv_94xx.c
index 7e423e5..e1f35d4 100644
--- a/drivers/scsi/mvsas/mv_94xx.c
+++ b/drivers/scsi/mvsas/mv_94xx.c
@@ -715,11 +715,13 @@ static void mvs_94xx_free_reg_set(struct mvs_info *mvi,
u8 *tfs)
if (*tfs == MVS_ID_NOT_MAPPED)
return;
- mvi-sata_reg_set = ~bit(reg_set);
- if (reg_set 32)
+ if (reg_set 32) {
+ mvi-sata_reg_set = ~bit(reg_set);
w_reg_set_enable(reg_set, (u32)mvi-sata_reg_set);
- else
+ } else if (reg_set 64) {
+ mvi-sata_reg_set = ~bit(reg_set);
w_reg_set_enable(reg_set, (u32)(mvi-sata_reg_set 32));
+ }
*tfs = MVS_ID_NOT_MAPPED;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
