Messages in this thread Patch in this message | | | Date | Wed, 08 Jan 2014 08:32:28 +0100 | From | Hannes Reinecke <> | Subject | Re: [PATCH] drivers: target: target_core_mod: use div64_u64_rem() instead of operator '%' for u64 |
| |
On 12/24/2013 04:35 AM, Chen Gang wrote: > On 12/23/2013 02:51 PM, Nicholas A. Bellinger wrote: >> On Sun, 2013-12-22 at 17:17 +0800, Chen Gang wrote: >>> On 12/22/2013 10:56 AM, Nicholas A. Bellinger wrote: >>>> Hi Chen, >>>> >>>> On Sat, 2013-12-21 at 10:08 +0800, Chen Gang wrote: >>>>> In kernel, need use div64_u64_rem() instead of operator '%' for u64, or >>>>> can not pass compiling (with allmodconfig under metag): >>>>> >>>>> MODPOST 2909 modules >>>>> ERROR: "__umoddi3" [drivers/target/target_core_mod.ko] undefined! >>>>> >>>>> Also need u64 type cast for u32 variable multiply u32 variable, or will >>>>> cause type overflow issue. >>>>> >>>>> >>>>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> >>>>> --- >>>>> drivers/target/target_core_alua.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>> >>>> FYI, this unsigned long long division in core_alua_state_lba_dependent() >>>> was fixed for 32-bit in linux-next >= 12192013 code. >>>> >>> >>> OK, thanks. >>> >>> The related fix patch changed "start_lba = lba % ..." to "start_lba = >>> lba / ...", and also assumed "segment_size * segment_mult" is still >>> within u32 (can not cause type over flow). >>> >>> I guess the original author already knew about them, and intended to do >>> like that (if not, please let me know, thanks). >>> >> >> Sorry, your correct that the original code is using modulo division to >> calculate start_lba. >> > > Oh, that's all right, (in fact, don't need sorry), I am not quite > familiar with the details, so need related member help check it. :-) > >> Hannes, can you please double check this below..? >> > > Please help check when have time, thanks. > I would even convert segment_size and segment_mult to u64, to ensure no overflows occur:
diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua .c index 9b1856d..54b1e52 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -477,8 +477,7 @@ static inline int core_alua_state_lba_dependent( u8 *alua_ascq) { struct se_device *dev = cmd->se_dev; - u32 segment_size, segment_mult, sectors; - u64 lba; + u64 segment_size, segment_mult, sectors, lba;
/* Only need to check for cdb actually containing LBAs */ if (!(cmd->se_cmd_flags & SCF_SCSI_DATA_CDB))
Other than that the sector_div() patch is correct.
Cheers,
Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |