2002-10-02 20:05:17

by Denis Vlasenko

[permalink] [raw]
Subject: [PATCH] cli()/sti() fix for drivers/net/ewrk3.c

These are my first patches for network drivers.
Please comment if I'm doing something wrong.
Compile tested. Testers with hw wanted.
--
vda

diff -u --recursive linux-2.5.38orig/drivers/net/ewrk3.c linux-2.5.38/drivers/net/ewrk3.c
--- linux-2.5.38orig/drivers/net/ewrk3.c Sun Sep 22 04:25:09 2002
+++ linux-2.5.38/drivers/net/ewrk3.c Wed Oct 2 01:29:31 2002
@@ -1631,6 +1631,7 @@
u_long iobase = dev->base_addr;
int i, j, status = 0;
u_char csr;
+ unsigned long flags;
union ewrk3_addr {
u_char addr[HASH_TABLE_LEN * ETH_ALEN];
u_short val[(HASH_TABLE_LEN * ETH_ALEN) >> 1];
@@ -1746,18 +1747,18 @@

break;
case EWRK3_GET_STATS: /* Get the driver statistics */
- cli();
+ spin_lock_irqsave(&lp->hw_lock, flags);
ioc->len = sizeof(lp->pktStats);
if (copy_to_user(ioc->data, &lp->pktStats, ioc->len))
status = -EFAULT;
- sti();
+ spin_unlock_irqrestore(&lp->hw_lock,flags);

break;
case EWRK3_CLR_STATS: /* Zero out the driver statistics */
if (capable(CAP_NET_ADMIN)) {
- cli();
+ spin_lock_irqsave(&lp->hw_lock, flags);
memset(&lp->pktStats, 0, sizeof(lp->pktStats));
- sti();
+ spin_unlock_irqrestore(&lp->hw_lock,flags);
} else {
status = -EPERM;
}


2002-10-02 20:40:35

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH] cli()/sti() fix for drivers/net/ewrk3.c

Denis Vlasenko <[email protected]> :
[...]
> diff -u --recursive linux-2.5.38orig/drivers/net/ewrk3.c linux-2.5.38/drivers/net/ewrk3.c
> --- linux-2.5.38orig/drivers/net/ewrk3.c Sun Sep 22 04:25:09 2002
> +++ linux-2.5.38/drivers/net/ewrk3.c Wed Oct 2 01:29:31 2002
[...]

The remarks done for drivers/net/depca.c patch apply equally as well.

--
Ueimor

2002-10-03 08:14:04

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] cli()/sti() fix for drivers/net/ewrk3.c

On 2 October 2002 18:45, Francois Romieu wrote:
> Denis Vlasenko <[email protected]> :
> [...]
>
> > diff -u --recursive linux-2.5.38orig/drivers/net/ewrk3.c
> > linux-2.5.38/drivers/net/ewrk3.c ---
> > linux-2.5.38orig/drivers/net/ewrk3.c Sun Sep 22 04:25:09 2002 +++
> > linux-2.5.38/drivers/net/ewrk3.c Wed Oct 2 01:29:31 2002
>
> [...]
>
> The remarks done for drivers/net/depca.c patch apply equally as well.

Fixes similar to depca.c applied :-)
Testers with hardware, anyone?
--
vda

diff -u --recursive linux-2.5.40org/drivers/net/ewrk3.c linux-2.5.40/drivers/net/ewrk3.c
--- linux-2.5.40org/drivers/net/ewrk3.c Tue Oct 1 05:06:58 2002
+++ linux-2.5.40/drivers/net/ewrk3.c Thu Oct 3 11:04:33 2002
@@ -930,6 +930,7 @@
spin_unlock(&lp->hw_lock);
}

+/* Called with lp->lock held */
static int ewrk3_rx(struct net_device *dev)
{
struct ewrk3_private *lp = (struct ewrk3_private *) dev->priv;
@@ -1055,8 +1056,9 @@
}

/*
- ** Buffer sent - check for TX buffer errors.
- */
+** Buffer sent - check for TX buffer errors.
+** Called with lp->lock held
+*/
static int ewrk3_tx(struct net_device *dev)
{
struct ewrk3_private *lp = (struct ewrk3_private *) dev->priv;
@@ -1631,6 +1633,7 @@
u_long iobase = dev->base_addr;
int i, j, status = 0;
u_char csr;
+ unsigned long flags;
union ewrk3_addr {
u_char addr[HASH_TABLE_LEN * ETH_ALEN];
u_short val[(HASH_TABLE_LEN * ETH_ALEN) >> 1];
@@ -1745,19 +1748,26 @@
}

break;
- case EWRK3_GET_STATS: /* Get the driver statistics */
- cli();
- ioc->len = sizeof(lp->pktStats);
- if (copy_to_user(ioc->data, &lp->pktStats, ioc->len))
- status = -EFAULT;
- sti();
+ case EWRK3_GET_STATS: { /* Get the driver statistics */
+ typeof(lp->pktStats) *tmp_stats =
+ kmalloc(sizeof(lp->pktStats), GFP_KERNEL);
+ if (!tmp_stats) return -ENOMEM;
+
+ spin_lock_irqsave(&lp->lock, flags);
+ memcpy(tmp_stats, &lp->pktStats, sizeof(lp->pktStats));
+ spin_unlock_irqrestore(&lp->lock, flags);

+ ioc->len = sizeof(lp->pktStats);
+ if (copy_to_user(ioc->data, tmp_stats, sizeof(lp->pktStats)))
+ status = -EFAULT;
+ kfree(tmp_stats);
break;
+ }
case EWRK3_CLR_STATS: /* Zero out the driver statistics */
if (capable(CAP_NET_ADMIN)) {
- cli();
+ spin_lock_irqsave(&lp->hw_lock, flags);
memset(&lp->pktStats, 0, sizeof(lp->pktStats));
- sti();
+ spin_unlock_irqrestore(&lp->hw_lock,flags);
} else {
status = -EPERM;
}

2002-10-03 09:20:16

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] cli()/sti() fix for drivers/net/ewrk3.c

On 3 October 2002 11:08, Denis Vlasenko wrote:
> On 2 October 2002 18:45, Francois Romieu wrote:
> > Denis Vlasenko <[email protected]> :
> > [...]
> >
> > > diff -u --recursive linux-2.5.38orig/drivers/net/ewrk3.c
> > > linux-2.5.38/drivers/net/ewrk3.c ---
> > > linux-2.5.38orig/drivers/net/ewrk3.c Sun Sep 22 04:25:09 2002 +++
> > > linux-2.5.38/drivers/net/ewrk3.c Wed Oct 2 01:29:31 2002
> >
> > [...]
> >
> > The remarks done for drivers/net/depca.c patch apply equally as
> > well.
>
> Fixes similar to depca.c applied :-)
> Testers with hardware, anyone?

It's not ->lock, it's ->hw_lock, stupid!
Denis "can't compile" Vlasenko :-(
--
vda

diff -u --recursive linux-2.5.40org/drivers/net/ewrk3.c linux-2.5.40/drivers/net/ewrk3.c
--- linux-2.5.40org/drivers/net/ewrk3.c Tue Oct 1 05:06:58 2002
+++ linux-2.5.40/drivers/net/ewrk3.c Thu Oct 3 12:09:46 2002
@@ -930,6 +930,7 @@
spin_unlock(&lp->hw_lock);
}

+/* Called with lp->hw_lock held */
static int ewrk3_rx(struct net_device *dev)
{
struct ewrk3_private *lp = (struct ewrk3_private *) dev->priv;
@@ -1055,8 +1056,9 @@
}

/*
- ** Buffer sent - check for TX buffer errors.
- */
+** Buffer sent - check for TX buffer errors.
+** Called with lp->hw_lock held
+*/
static int ewrk3_tx(struct net_device *dev)
{
struct ewrk3_private *lp = (struct ewrk3_private *) dev->priv;
@@ -1631,6 +1633,7 @@
u_long iobase = dev->base_addr;
int i, j, status = 0;
u_char csr;
+ unsigned long flags;
union ewrk3_addr {
u_char addr[HASH_TABLE_LEN * ETH_ALEN];
u_short val[(HASH_TABLE_LEN * ETH_ALEN) >> 1];
@@ -1745,19 +1748,26 @@
}

break;
- case EWRK3_GET_STATS: /* Get the driver statistics */
- cli();
- ioc->len = sizeof(lp->pktStats);
- if (copy_to_user(ioc->data, &lp->pktStats, ioc->len))
- status = -EFAULT;
- sti();
+ case EWRK3_GET_STATS: { /* Get the driver statistics */
+ typeof(lp->pktStats) *tmp_stats =
+ kmalloc(sizeof(lp->pktStats), GFP_KERNEL);
+ if (!tmp_stats) return -ENOMEM;
+
+ spin_lock_irqsave(&lp->hw_lock, flags);
+ memcpy(tmp_stats, &lp->pktStats, sizeof(lp->pktStats));
+ spin_unlock_irqrestore(&lp->hw_lock, flags);

+ ioc->len = sizeof(lp->pktStats);
+ if (copy_to_user(ioc->data, tmp_stats, sizeof(lp->pktStats)))
+ status = -EFAULT;
+ kfree(tmp_stats);
break;
+ }
case EWRK3_CLR_STATS: /* Zero out the driver statistics */
if (capable(CAP_NET_ADMIN)) {
- cli();
+ spin_lock_irqsave(&lp->hw_lock, flags);
memset(&lp->pktStats, 0, sizeof(lp->pktStats));
- sti();
+ spin_unlock_irqrestore(&lp->hw_lock,flags);
} else {
status = -EPERM;
}