[PATCH 0/2] Fix sloppy typing in the fast drain logic

[PATCH 0/2] Fix sloppy typing in the fast drain logic

[Sergey Shtylyov]

Here are 2 patches against the 'for-next' branch of Damien Le Moal's
'libata.git' repo.
The libata's fast drain logic unfortunately boasts of some sloppy typing --
unify it by using *unsigned int* everyhere...

Sergey Shtylyov (2):
  ata: libata-eh: fix sloppy result type of ata_eh_nr_in_flight()
  ata: make ata_port::fastdrain_cnt *unsigned int*

 drivers/ata/libata-eh.c | 8 ++++----
 include/linux/libata.h  | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.26.3

[PATCH 1/2] ata: libata-eh: fix sloppy result type of ata_eh_nr_in_flight()

[Sergey Shtylyov]

ata_eh_nr_in_flight() counts the # of the active tagged commands and thus
cannot return a negative value but the result type is nevertheless *int*;
switching it to *unsigned int* (along with the local variables receiving
the function's result) helps avoiding the sign extension instructions when
comparing with or assigning to *unsigned long *ata_port::fastdrain_cnt and
thus results in a more compact 64-bit code...

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
 drivers/ata/libata-eh.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 3307ed45fe4d..25586e16692d 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -802,11 +802,11 @@ void ata_port_wait_eh(struct ata_port *ap)
 }
 EXPORT_SYMBOL_GPL(ata_port_wait_eh);
 
-static int ata_eh_nr_in_flight(struct ata_port *ap)
+static unsigned int ata_eh_nr_in_flight(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc;
 	unsigned int tag;
-	int nr = 0;
+	unsigned int nr = 0;
 
 	/* count only non-internal commands */
 	ata_qc_for_each(ap, qc, tag) {
@@ -821,7 +821,7 @@ void ata_eh_fastdrain_timerfn(struct timer_list *t)
 {
 	struct ata_port *ap = from_timer(ap, t, fastdrain_timer);
 	unsigned long flags;
-	int cnt;
+	unsigned int cnt;
 
 	spin_lock_irqsave(ap->lock, flags);
 
@@ -870,7 +870,7 @@ void ata_eh_fastdrain_timerfn(struct timer_list *t)
  */
 static void ata_eh_set_pending(struct ata_port *ap, int fastdrain)
 {
-	int cnt;
+	unsigned int cnt;
 
 	/* already scheduled? */
 	if (ap->pflags & ATA_PFLAG_EH_PENDING)
-- 
2.26.3

Re: [PATCH 1/2] ata: libata-eh: fix sloppy result type of ata_eh_nr_in_flight()

[Sergey Shtylyov]

On 6/16/22 11:51 PM, Sergey Shtylyov wrote:

> ata_eh_nr_in_flight() counts the # of the active tagged commands and thus
> cannot return a negative value but the result type is nevertheless *int*;
> switching it to *unsigned int* (along with the local variables receiving
> the function's result) helps avoiding the sign extension instructions when
> comparing with or assigning to *unsigned long *ata_port::fastdrain_cnt and

   Grr, a typo! Should have been *unsigned long* ata_port::fastdrain_cnt...
Damien, coyld you please fix it?

> thus results in a more compact 64-bit code...
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
[...]

MBR, Sergey

Re: [PATCH 1/2] ata: libata-eh: fix sloppy result type of ata_eh_nr_in_flight()

[Damien Le Moal]

On 6/18/22 03:16, Sergey Shtylyov wrote:
> On 6/16/22 11:51 PM, Sergey Shtylyov wrote:
> 
>> ata_eh_nr_in_flight() counts the # of the active tagged commands and thus
>> cannot return a negative value but the result type is nevertheless *int*;
>> switching it to *unsigned int* (along with the local variables receiving
>> the function's result) helps avoiding the sign extension instructions when
>> comparing with or assigning to *unsigned long *ata_port::fastdrain_cnt and
> 
>    Grr, a typo! Should have been *unsigned long* ata_port::fastdrain_cnt...
> Damien, coyld you please fix it?

OK. Note: please stop using markdown stuff like *xxx*. That is not useful
and that will avoid this kind of typos.

> 
>> thus results in a more compact 64-bit code...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> [...]
> 
> MBR, Sergey


-- 
Damien Le Moal
Western Digital Research

[PATCH 2/2] ata: make ata_port::fastdrain_cnt *unsigned int*

[Sergey Shtylyov]

*unsigned long* ata_port::fastdrain_cnt (64-bit value in a 64-bit kernel)
is always assigned from the 32-bit *unsigned int* variables, thus could
also be made just *unsigned int*...

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
 include/linux/libata.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0f2a59c9c735..d214aa988168 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -850,7 +850,7 @@ struct ata_port {
 	enum ata_lpm_policy	target_lpm_policy;
 
 	struct timer_list	fastdrain_timer;
-	unsigned long		fastdrain_cnt;
+	unsigned int		fastdrain_cnt;
 
 	async_cookie_t		cookie;
 
-- 
2.26.3

Re: [PATCH 0/2] Fix sloppy typing in the fast drain logic

[Damien Le Moal]

On 6/17/22 05:51, Sergey Shtylyov wrote:
> Here are 2 patches against the 'for-next' branch of Damien Le Moal's
> 'libata.git' repo.
> The libata's fast drain logic unfortunately boasts of some sloppy typing --
> unify it by using *unsigned int* everyhere...
> 
> Sergey Shtylyov (2):
>   ata: libata-eh: fix sloppy result type of ata_eh_nr_in_flight()
>   ata: make ata_port::fastdrain_cnt *unsigned int*
> 
>  drivers/ata/libata-eh.c | 8 ++++----
>  include/linux/libata.h  | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 

Applied to for-5.20. Thanks !

-- 
Damien Le Moal
Western Digital Research