* [PATCH] atl1: fix oops when changing tx/rx ring params
@ 2010-12-27 15:54 J. K. Cliburn
2010-12-31 20:08 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: J. K. Cliburn @ 2010-12-27 15:54 UTC (permalink / raw)
To: David Miller
Cc: Tõnu Raitviir, Luca Tettamanti, Chris Snook, netdev,
Xiong Huang, stable
Commit 3f5a2a713aad28480d86b0add00c68484b54febc zeroes out the statistics
message block (SMB) and coalescing message block (CMB) when adapter ring
resources are freed. This is desirable behavior, but, as a side effect,
the commit leads to an oops when atl1_set_ringparam() attempts to alter
the number of rx or tx descriptors in the ring buffer (by using ethtool
-G, for example). We don't want SMB or CMB to change during this
operation.
Modify atl1_set_ringparam() to preserve SMB and CMB when changing ring
parameters.
Cc: stable@kernel.org
Signed-off-by: Jay Cliburn <jcliburn@gmail.com>
Reported-by: Tõnu Raitviir <jussuf@linux.ee>
---
drivers/net/atlx/atl1.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index def8df8..6d4f051 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -3504,6 +3504,8 @@ static int atl1_set_ringparam(struct net_device *netdev,
struct atl1_rfd_ring rfd_old, rfd_new;
struct atl1_rrd_ring rrd_old, rrd_new;
struct atl1_ring_header rhdr_old, rhdr_new;
+ struct atl1_smb smb;
+ struct atl1_cmb cmb;
int err;
tpd_old = adapter->tpd_ring;
@@ -3544,11 +3546,20 @@ static int atl1_set_ringparam(struct net_device *netdev,
adapter->rrd_ring = rrd_old;
adapter->tpd_ring = tpd_old;
adapter->ring_header = rhdr_old;
+ /*
+ * Save SMB and CMB, since atl1_free_ring_resources
+ * will clear them as a result of commit
+ * 3f5a2a713aad28480d86b0add00c68484b54febc
+ */
+ smb = adapter->smb;
+ cmb = adapter->cmb;
atl1_free_ring_resources(adapter);
adapter->rfd_ring = rfd_new;
adapter->rrd_ring = rrd_new;
adapter->tpd_ring = tpd_new;
adapter->ring_header = rhdr_new;
+ adapter->smb = smb;
+ adapter->cmb = cmb;
err = atl1_up(adapter);
if (err)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] atl1: fix oops when changing tx/rx ring params
2010-12-27 15:54 [PATCH] atl1: fix oops when changing tx/rx ring params J. K. Cliburn
@ 2010-12-31 20:08 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-12-31 20:08 UTC (permalink / raw)
To: jcliburn; +Cc: jussuf, kronos.it, chris.snook, netdev, Xiong.Huang, stable
From: "J. K. Cliburn" <jcliburn@gmail.com>
Date: Mon, 27 Dec 2010 09:54:02 -0600
> @@ -3544,11 +3546,20 @@ static int atl1_set_ringparam(struct net_device *netdev,
> adapter->rrd_ring = rrd_old;
> adapter->tpd_ring = tpd_old;
> adapter->ring_header = rhdr_old;
> + /*
> + * Save SMB and CMB, since atl1_free_ring_resources
> + * will clear them as a result of commit
> + * 3f5a2a713aad28480d86b0add00c68484b54febc
> + */
Please do not reference commit IDs in code comments.
Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] atl1: fix oops when changing tx/rx ring params
@ 2011-01-01 15:02 J. K. Cliburn
2011-01-03 19:04 ` David Miller
2011-01-05 15:42 ` Luca Tettamanti
0 siblings, 2 replies; 6+ messages in thread
From: J. K. Cliburn @ 2011-01-01 15:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev, stable, jussuf, chris.snook, kronos.it, Xiong.Huang
Commit 3f5a2a713aad28480d86b0add00c68484b54febc zeroes out the statistics
message block (SMB) and coalescing message block (CMB) when adapter ring
resources are freed. This is desirable behavior, but, as a side effect,
the commit leads to an oops when atl1_set_ringparam() attempts to alter
the number of rx or tx elements in the ring buffer (by using ethtool
-G, for example). We don't want SMB or CMB to change during this
operation.
Modify atl1_set_ringparam() to preserve SMB and CMB when changing ring
parameters.
Cc: stable@kernel.org
Signed-off-by: Jay Cliburn <jcliburn@gmail.com>
Reported-by: Tõnu Raitviir <jussuf@linux.ee>
---
drivers/net/atlx/atl1.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 5336310..3acf512 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -3504,6 +3504,8 @@ static int atl1_set_ringparam(struct net_device *netdev,
struct atl1_rfd_ring rfd_old, rfd_new;
struct atl1_rrd_ring rrd_old, rrd_new;
struct atl1_ring_header rhdr_old, rhdr_new;
+ struct atl1_smb smb;
+ struct atl1_cmb cmb;
int err;
tpd_old = adapter->tpd_ring;
@@ -3544,11 +3546,19 @@ static int atl1_set_ringparam(struct net_device *netdev,
adapter->rrd_ring = rrd_old;
adapter->tpd_ring = tpd_old;
adapter->ring_header = rhdr_old;
+ /*
+ * Save SMB and CMB, since atl1_free_ring_resources
+ * will clear them.
+ */
+ smb = adapter->smb;
+ cmb = adapter->cmb;
atl1_free_ring_resources(adapter);
adapter->rfd_ring = rfd_new;
adapter->rrd_ring = rrd_new;
adapter->tpd_ring = tpd_new;
adapter->ring_header = rhdr_new;
+ adapter->smb = smb;
+ adapter->cmb = cmb;
err = atl1_up(adapter);
if (err)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] atl1: fix oops when changing tx/rx ring params
2011-01-01 15:02 J. K. Cliburn
@ 2011-01-03 19:04 ` David Miller
2011-01-05 15:42 ` Luca Tettamanti
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2011-01-03 19:04 UTC (permalink / raw)
To: jcliburn; +Cc: netdev, stable, jussuf, chris.snook, kronos.it, Xiong.Huang
From: "J. K. Cliburn" <jcliburn@gmail.com>
Date: Sat, 1 Jan 2011 09:02:12 -0600
> Commit 3f5a2a713aad28480d86b0add00c68484b54febc zeroes out the statistics
> message block (SMB) and coalescing message block (CMB) when adapter ring
> resources are freed. This is desirable behavior, but, as a side effect,
> the commit leads to an oops when atl1_set_ringparam() attempts to alter
> the number of rx or tx elements in the ring buffer (by using ethtool
> -G, for example). We don't want SMB or CMB to change during this
> operation.
>
> Modify atl1_set_ringparam() to preserve SMB and CMB when changing ring
> parameters.
>
> Cc: stable@kernel.org
> Signed-off-by: Jay Cliburn <jcliburn@gmail.com>
> Reported-by: Tõnu Raitviir <jussuf@linux.ee>
I'll apply this, thanks Jay.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] atl1: fix oops when changing tx/rx ring params
2011-01-01 15:02 J. K. Cliburn
2011-01-03 19:04 ` David Miller
@ 2011-01-05 15:42 ` Luca Tettamanti
2011-01-05 15:45 ` Luca Tettamanti
1 sibling, 1 reply; 6+ messages in thread
From: Luca Tettamanti @ 2011-01-05 15:42 UTC (permalink / raw)
To: J. K. Cliburn
Cc: David Miller, netdev, stable, jussuf, chris.snook, Xiong.Huang
On Sat, Jan 1, 2011 at 4:02 PM, J. K. Cliburn <jcliburn@gmail.com> wrote:
> Commit 3f5a2a713aad28480d86b0add00c68484b54febc zeroes out the statistics
> message block (SMB) and coalescing message block (CMB) when adapter ring
> resources are freed. This is desirable behavior, but, as a side effect,
> the commit leads to an oops when atl1_set_ringparam() attempts to alter
> the number of rx or tx elements in the ring buffer (by using ethtool
> -G, for example). We don't want SMB or CMB to change during this
> operation.
>
> Modify atl1_set_ringparam() to preserve SMB and CMB when changing ring
> parameters.
>
> Cc: stable@kernel.org
> Signed-off-by: Jay Cliburn <jcliburn@gmail.com>
> Reported-by: Tõnu Raitviir <jussuf@linux.ee>
> ---
> drivers/net/atlx/atl1.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
> index 5336310..3acf512 100644
> --- a/drivers/net/atlx/atl1.c
> +++ b/drivers/net/atlx/atl1.c
> @@ -3504,6 +3504,8 @@ static int atl1_set_ringparam(struct net_device *netdev,
> struct atl1_rfd_ring rfd_old, rfd_new;
> struct atl1_rrd_ring rrd_old, rrd_new;
> struct atl1_ring_header rhdr_old, rhdr_new;
> + struct atl1_smb smb;
> + struct atl1_cmb cmb;
> int err;
>
> tpd_old = adapter->tpd_ring;
> @@ -3544,11 +3546,19 @@ static int atl1_set_ringparam(struct net_device *netdev,
> adapter->rrd_ring = rrd_old;
> adapter->tpd_ring = tpd_old;
> adapter->ring_header = rhdr_old;
> + /*
> + * Save SMB and CMB, since atl1_free_ring_resources
> + * will clear them.
> + */
> + smb = adapter->smb;
> + cmb = adapter->cmb;
> atl1_free_ring_resources(adapter);
Hum, unless I'm missing something atl1_free_ring_resources frees the
whole ring_header->dma block which contains both SMB and CMB.
> adapter->rfd_ring = rfd_new;
> adapter->rrd_ring = rrd_new;
> adapter->tpd_ring = tpd_new;
> adapter->ring_header = rhdr_new;
> + adapter->smb = smb;
> + adapter->cmb = cmb;
So here you're using pointers to freed memory.
In order to preserve the stats you'd have to copy the structure.
Luca
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] atl1: fix oops when changing tx/rx ring params
2011-01-05 15:42 ` Luca Tettamanti
@ 2011-01-05 15:45 ` Luca Tettamanti
0 siblings, 0 replies; 6+ messages in thread
From: Luca Tettamanti @ 2011-01-05 15:45 UTC (permalink / raw)
To: J. K. Cliburn
Cc: David Miller, netdev, stable, jussuf, chris.snook, Xiong.Huang
On Wed, Jan 5, 2011 at 4:42 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> So here you're using pointers to freed memory.
> In order to preserve the stats you'd have to copy the structure.
Doh. I still haven't recovered from all the partying ;-)
Sorry for the noise...
L
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-05 15:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-27 15:54 [PATCH] atl1: fix oops when changing tx/rx ring params J. K. Cliburn
2010-12-31 20:08 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2011-01-01 15:02 J. K. Cliburn
2011-01-03 19:04 ` David Miller
2011-01-05 15:42 ` Luca Tettamanti
2011-01-05 15:45 ` Luca Tettamanti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).