Subject: Re: [PATCH -next] net: mvpp2: Use div64_ul instead of do_div
To:     Yang Li <yang.lee@linux.alibaba.com>, davem@davemloft.net
Cc:     kuba@kernel.org, mw@semihalf.com, linux@armlinux.org.uk,
        ast@kernel.org, daniel@iogearbox.net, hawk@kernel.org,
        john.fastabend@gmail.com, andrii@kernel.org, kafai@fb.com,
        songliubraving@fb.com, yhs@fb.com, kpsingh@kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        bpf@vger.kernel.org
References: <1637204701-8224-1-git-send-email-yang.lee@linux.alibaba.com>
From:   Eric Dumazet <eric.dumazet@gmail.com>
Message-ID: <cbcdb354-04de-2a9a-1754-c32dd014e859@gmail.com>
Date:   Wed, 17 Nov 2021 20:00:35 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <1637204701-8224-1-git-send-email-yang.lee@linux.alibaba.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 11/17/21 7:05 PM, Yang Li wrote:
> do_div() does a 64-by-32 division. Here the divisor is an
> unsigned long which on some platforms is 64 bit wide. So use
> div64_ul instead of do_div to avoid a possible truncation.
> 
> Eliminate the following coccicheck warning:
> ./drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:2742:1-7: WARNING:
> do_div() does a 64-by-32 division, please consider using div64_ul
> instead.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index df6c793..41244a5 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -2739,7 +2739,7 @@ static u32 mvpp2_cycles_to_usec(u32 cycles, unsigned long clk_hz)
>  {
>  	u64 tmp = (u64)cycles * USEC_PER_SEC;
>  
> -	do_div(tmp, clk_hz);
> +	tmp = div64_ul(tmp, clk_hz);
>  
>  	return tmp > U32_MAX ? U32_MAX : tmp;
>  }
> 

This is silly, clk_hz fits in a u32, why pretends it is 64bit ?

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index df6c793f4b1b04eb1f812ce2a3a82a39cde0f68b..1bea316bf13ac42ad65ba326fc1d9e206546766e 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -2726,7 +2726,7 @@ static void mvpp2_tx_pkts_coal_set(struct mvpp2_port *port,
        }
 }
 
-static u32 mvpp2_usec_to_cycles(u32 usec, unsigned long clk_hz)
+static u32 mvpp2_usec_to_cycles(u32 usec, u32 clk_hz)
 {
        u64 tmp = (u64)clk_hz * usec;
 
@@ -2735,7 +2735,7 @@ static u32 mvpp2_usec_to_cycles(u32 usec, unsigned long clk_hz)
        return tmp > U32_MAX ? U32_MAX : tmp;
 }
 
-static u32 mvpp2_cycles_to_usec(u32 cycles, unsigned long clk_hz)
+static u32 mvpp2_cycles_to_usec(u32 cycles, u32 clk_hz)
 {
        u64 tmp = (u64)cycles * USEC_PER_SEC;
 
@@ -2748,7 +2748,7 @@ static u32 mvpp2_cycles_to_usec(u32 cycles, unsigned long clk_hz)
 static void mvpp2_rx_time_coal_set(struct mvpp2_port *port,
                                   struct mvpp2_rx_queue *rxq)
 {
-       unsigned long freq = port->priv->tclk;
+       u32 freq = port->priv->tclk;
        u32 val = mvpp2_usec_to_cycles(rxq->time_coal, freq);
 
        if (val > MVPP2_MAX_ISR_RX_THRESHOLD) {
@@ -2764,7 +2764,7 @@ static void mvpp2_rx_time_coal_set(struct mvpp2_port *port,
 
 static void mvpp2_tx_time_coal_set(struct mvpp2_port *port)
 {
-       unsigned long freq = port->priv->tclk;
+       u32 freq = port->priv->tclk;
        u32 val = mvpp2_usec_to_cycles(port->tx_time_coal, freq);
 
        if (val > MVPP2_MAX_ISR_TX_THRESHOLD) {