diff mbox series

[net] qede: fix interrupt coalescing configuration

Message ID 20230216115447.17227-1-manishc@marvell.com (mailing list archive)
State Accepted
Commit 908d4bb7c54caa58253a363d63e797a468eaf321
Delegated to: Netdev Maintainers
Headers show
Series [net] qede: fix interrupt coalescing configuration | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers fail 1 blamed authors not CCed: irusskikh@marvell.com; 3 maintainers not CCed: edumazet@google.com irusskikh@marvell.com pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 17 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Manish Chopra Feb. 16, 2023, 11:54 a.m. UTC 
On default driver load device gets configured with unexpected
higher interrupt coalescing values instead of default expected
values as memory allocated from krealloc() is not supposed to
be zeroed out and may contain garbage values.

Fix this by allocating the memory of required size first with
kcalloc() and then use krealloc() to resize and preserve the
contents across down/up of the interface.

Signed-off-by: Manish Chopra <manishc@marvell.com>
Fixes: b0ec5489c480 ("qede: preserve per queue stats across up/down of interface")
Cc: stable@vger.kernel.org
Cc: Bhaskar Upadhaya <bupadhaya@marvell.com>
Cc: David S. Miller <davem@davemloft.net>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2160054
Signed-off-by: Alok Prasad <palok@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qede/qede_main.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

David Laight Feb. 16, 2023, 12:40 p.m. UTC | #1 
From: Manish Chopra
> Sent: 16 February 2023 11:55
> 
> On default driver load device gets configured with unexpected
> higher interrupt coalescing values instead of default expected
> values as memory allocated from krealloc() is not supposed to
> be zeroed out and may contain garbage values.
> 
> Fix this by allocating the memory of required size first with
> kcalloc() and then use krealloc() to resize and preserve the
> contents across down/up of the interface.

Doesn't any extra memory allocated by krealloc() need to
be zerod ?

	David

> 
> Signed-off-by: Manish Chopra <manishc@marvell.com>
> Fixes: b0ec5489c480 ("qede: preserve per queue stats across up/down of interface")
> Cc: stable@vger.kernel.org
> Cc: Bhaskar Upadhaya <bupadhaya@marvell.com>
> Cc: David S. Miller <davem@davemloft.net>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2160054
> Signed-off-by: Alok Prasad <palok@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>
> ---
>  drivers/net/ethernet/qlogic/qede/qede_main.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c
> b/drivers/net/ethernet/qlogic/qede/qede_main.c
> index 953f304b8588..af39513db1ba 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> @@ -970,8 +970,15 @@ static int qede_alloc_fp_array(struct qede_dev *edev)
>  		goto err;
>  	}
> 
> -	mem = krealloc(edev->coal_entry, QEDE_QUEUE_CNT(edev) *
> -		       sizeof(*edev->coal_entry), GFP_KERNEL);
> +	if (!edev->coal_entry) {
> +		mem = kcalloc(QEDE_MAX_RSS_CNT(edev),
> +			      sizeof(*edev->coal_entry), GFP_KERNEL);
> +	} else {
> +		mem = krealloc(edev->coal_entry,
> +			       QEDE_QUEUE_CNT(edev) * sizeof(*edev->coal_entry),
> +			       GFP_KERNEL);
> +	}
> +
>  	if (!mem) {
>  		DP_ERR(edev, "coalesce entry allocation failed\n");
>  		kfree(edev->coal_entry);
> --
> 2.27.0

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Manish Chopra Feb. 16, 2023, 3:06 p.m. UTC | #2 
> -----Original Message-----
> From: David Laight <David.Laight@ACULAB.COM>
> Sent: Thursday, February 16, 2023 6:11 PM
> To: Manish Chopra <manishc@marvell.com>; kuba@kernel.org
> Cc: netdev@vger.kernel.org; Ariel Elior <aelior@marvell.com>;
> stable@vger.kernel.org; Bhaskar Upadhaya <bupadhaya@marvell.com>;
> David S . Miller <davem@davemloft.net>
> Subject: [EXT] RE: [PATCH net] qede: fix interrupt coalescing configuration
> 
> External Email
> 
> ----------------------------------------------------------------------
> From: Manish Chopra
> > Sent: 16 February 2023 11:55
> >
> > On default driver load device gets configured with unexpected higher
> > interrupt coalescing values instead of default expected values as
> > memory allocated from krealloc() is not supposed to be zeroed out and
> > may contain garbage values.
> >
> > Fix this by allocating the memory of required size first with
> > kcalloc() and then use krealloc() to resize and preserve the contents
> > across down/up of the interface.
> 
> Doesn't any extra memory allocated by krealloc() need to be zerod ?

Hi David,

Driver will not attempt to allocate the size (using krealloc()) which is greater than the size
initially allocated by kcalloc() ever.

Thanks,
Manish

> 
> 	David
> 
> >
> > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > Fixes: b0ec5489c480 ("qede: preserve per queue stats across up/down of
> > interface")
> > Cc: stable@vger.kernel.org
> > Cc: Bhaskar Upadhaya <bupadhaya@marvell.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Link:
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.redhat.c
> > om_show-5Fbug.cgi-3Fid-
> 3D2160054&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=b
> > MTgx2X48QVXyXOEL8ALyI4dsWoR-m74c5n3d-
> ruJI8&m=yxUlxYqV9xzhL4ZcgCffdrxKT
> >
> 6qVQqXE6JdE28XtyOwxInnkbtBiM1lxjYJCI0Wm&s=uz96PudLovEFy3W0_IpBM
> wBWwGkl
> > 88SQR0pJXP0k-5I&e=
> > Signed-off-by: Alok Prasad <palok@marvell.com>
> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> > ---
> >  drivers/net/ethernet/qlogic/qede/qede_main.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c
> > b/drivers/net/ethernet/qlogic/qede/qede_main.c
> > index 953f304b8588..af39513db1ba 100644
> > --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> > +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> > @@ -970,8 +970,15 @@ static int qede_alloc_fp_array(struct qede_dev
> *edev)
> >  		goto err;
> >  	}
> >
> > -	mem = krealloc(edev->coal_entry, QEDE_QUEUE_CNT(edev) *
> > -		       sizeof(*edev->coal_entry), GFP_KERNEL);
> > +	if (!edev->coal_entry) {
> > +		mem = kcalloc(QEDE_MAX_RSS_CNT(edev),
> > +			      sizeof(*edev->coal_entry), GFP_KERNEL);
> > +	} else {
> > +		mem = krealloc(edev->coal_entry,
> > +			       QEDE_QUEUE_CNT(edev) * sizeof(*edev-
> >coal_entry),
> > +			       GFP_KERNEL);
> > +	}
> > +
> >  	if (!mem) {
> >  		DP_ERR(edev, "coalesce entry allocation failed\n");
> >  		kfree(edev->coal_entry);
> > --
> > 2.27.0
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
> MK1 1PT, UK Registration No: 1397386 (Wales)
patchwork-bot+netdevbpf@kernel.org Feb. 20, 2023, 8:30 a.m. UTC | #3 
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 16 Feb 2023 03:54:47 -0800 you wrote:
> On default driver load device gets configured with unexpected
> higher interrupt coalescing values instead of default expected
> values as memory allocated from krealloc() is not supposed to
> be zeroed out and may contain garbage values.
> 
> Fix this by allocating the memory of required size first with
> kcalloc() and then use krealloc() to resize and preserve the
> contents across down/up of the interface.
> 
> [...]

Here is the summary with links:
  - [net] qede: fix interrupt coalescing configuration
    https://git.kernel.org/netdev/net/c/908d4bb7c54c

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 953f304b8588..af39513db1ba 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -970,8 +970,15 @@  static int qede_alloc_fp_array(struct qede_dev *edev)
 		goto err;
 	}
 
-	mem = krealloc(edev->coal_entry, QEDE_QUEUE_CNT(edev) *
-		       sizeof(*edev->coal_entry), GFP_KERNEL);
+	if (!edev->coal_entry) {
+		mem = kcalloc(QEDE_MAX_RSS_CNT(edev),
+			      sizeof(*edev->coal_entry), GFP_KERNEL);
+	} else {
+		mem = krealloc(edev->coal_entry,
+			       QEDE_QUEUE_CNT(edev) * sizeof(*edev->coal_entry),
+			       GFP_KERNEL);
+	}
+
 	if (!mem) {
 		DP_ERR(edev, "coalesce entry allocation failed\n");
 		kfree(edev->coal_entry);