Hello! On 6/17/22 10:44 AM, Damien Le Moal wrote: >> Make the 'timeout' parameter to ata_exec_internal_sg() *unsigned int* as >> msecs_to_jiffies() that it calls takes just *unsigned int* for the time in >> milliseconds. Then follow the suit with ata_exec_internal(), its only >> caller; also fix up ata_dev_set_feature(), the only ata_exec_internal()'s >> caller that explicitly passes *unsigned long* variable for timeout... > > Checking this, struct ata_eh_cmd_timeout_ent uses an unsigned long timeout > and ata_internal_cmd_timeout() returns an unsigned long which is stored > into the unsigned int timeout variable. I know. All timeouts stored in those tables fit into *unsigned int* (except the last ones which get truncated from ULONG_MAX to UINT_MAX anyways). Note that *unsigned long* has always been silently truncated to *unsigned int* when calling msecs_to_jiffies() in ata_exec_internal_sg(); nothing changes with my patch, it just gets truncated a bit earlier... > So it may be good to add another > prep patch before this one to cleanup the auto_timeout stuff (struct > ata_eh_cmd_timeout_ent and ata_internal_cmd_timeout()). It shouldn't matter whether we do it before or after this patch, due to truncation which happens even now (I have such a patch, but no description yet). I was planning to address that as a part of the large patch series, most probably next week (and I wrote about that in the cover letter)... > Hmm ? Thoughts ? Was I clear enough? :-) >> Found by Linux Verification Center (linuxtesting.org) with the SVACE static >> analysis tool. I wanted to fix SVACE's reports 1st, then the rest of the sloppy timeout typing... >> >> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx> [...] MBR, Sergey
