| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <alpine.LFD.2.00.0908211304000.6044@xanadu.home>
Date: Fri, 21 Aug 2009 13:13:32 -0400 (EDT)
From: Nicolas Pitre <nico@....org>
To: DDD <Dongdong.deng@...driver.com>
Cc: Matt Mackall <mpm@...enic.com>, David Miller <davem@...emloft.net>,
lkml <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org,
Jason Wessel <jason.wessel@...driver.com>
Subject: Re: [RFC][PATCH] smc91x: let smc91x work well with netpoll
On Fri, 21 Aug 2009, DDD wrote:
> This patch changes too much and I didn't have the environment to test, so it is unverified patch.
> That's why I separate it from my previous patch, and send it solely as a RFC patch.
>
>
> Hi Nicolas,
> Given that you are the maintainer of "smc91x" driver since 2004. Could you say somethings about this
> patch? Your input on this patch is greatly appreciated. :-)
Looks fine to me. The most significant changes affect usage of this
driver on a SMP system. I'm afraid those might be hard to find now
(this network chip is already a strange piece of hardware, and I knew
about only one platform with it being SMP), so the best I can do is
review your patch only.
Which I now did and you can add my:
Acked-by: Nicolas Pitre <nico@....org>
> Thank you very much,
> Dongdong
>
>
> Patch Note:
> @@ -520,21 +522,21
> - local_irq_disable(); \
> + local_irq_save(flags); \
> __ret = spin_trylock(lock); \
> if (!__ret) \
> - local_irq_enable(); \
> + local_irq_restore(flags); \
>
> Here, for "__ret = spin_trylock(lock)", I didn't use
> spin_trylock_irqsave() to replace spin_trylock(), because
> the current irq state have got by local_irq_save(flags).
>
>
>
> ---
> drivers/net/smc91x.c | 40 ++++++++++++++++++++++------------------
> 1 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/smc91x.c b/drivers/net/smc91x.c
> index 1c70e99..7cabea1 100644
> --- a/drivers/net/smc91x.c
> +++ b/drivers/net/smc91x.c
> @@ -196,21 +196,23 @@ static void PRINT_PKT(u_char *buf, int length)
> /* this enables an interrupt in the interrupt mask register */
> #define SMC_ENABLE_INT(lp, x) do { \
> unsigned char mask; \
> - spin_lock_irq(&lp->lock); \
> + unsigned long smc_enable_flags; \
> + spin_lock_irqsave(&lp->lock, smc_int_flags); \
> mask = SMC_GET_INT_MASK(lp); \
> mask |= (x); \
> SMC_SET_INT_MASK(lp, mask); \
> - spin_unlock_irq(&lp->lock); \
> + spin_unlock_irqrestore(&lp->lock, smc_int_flags); \
> } while (0)
>
> /* this disables an interrupt from the interrupt mask register */
> #define SMC_DISABLE_INT(lp, x) do { \
> unsigned char mask; \
> - spin_lock_irq(&lp->lock); \
> + unsigned long smc_disable_flags; \
> + spin_lock_irqsave(&lp->lock, smc_disable_flags); \
> mask = SMC_GET_INT_MASK(lp); \
> mask &= ~(x); \
> SMC_SET_INT_MASK(lp, mask); \
> - spin_unlock_irq(&lp->lock); \
> + spin_unlock_irqrestore(&lp->lock, smc_disable_flags); \
> } while (0)
>
> /*
> @@ -520,21 +522,21 @@ static inline void smc_rcv(struct net_device *dev)
> * any other concurrent access and C would always interrupt B. But life
> * isn't that easy in a SMP world...
> */
> -#define smc_special_trylock(lock) \
> +#define smc_special_trylock(lock, flags) \
> ({ \
> int __ret; \
> - local_irq_disable(); \
> + local_irq_save(flags); \
> __ret = spin_trylock(lock); \
> if (!__ret) \
> - local_irq_enable(); \
> + local_irq_restore(flags); \
> __ret; \
> })
> -#define smc_special_lock(lock) spin_lock_irq(lock)
> -#define smc_special_unlock(lock) spin_unlock_irq(lock)
> +#define smc_special_lock(lock, flags) spin_lock_irq(lock, flags)
> +#define smc_special_unlock(lock, flags) spin_unlock_irqrestore(lock, flags)
> #else
> -#define smc_special_trylock(lock) (1)
> -#define smc_special_lock(lock) do { } while (0)
> -#define smc_special_unlock(lock) do { } while (0)
> +#define smc_special_trylock(lock, flags) (1)
> +#define smc_special_lock(lock, flags) do { } while (0)
> +#define smc_special_unlock(lock, flags) do { } while (0)
> #endif
>
> /*
> @@ -548,10 +550,11 @@ static void smc_hardware_send_pkt(unsigned long data)
> struct sk_buff *skb;
> unsigned int packet_no, len;
> unsigned char *buf;
> + unsigned long flags;
>
> DBG(3, "%s: %s\n", dev->name, __func__);
>
> - if (!smc_special_trylock(&lp->lock)) {
> + if (!smc_special_trylock(&lp->lock, flags)) {
> netif_stop_queue(dev);
> tasklet_schedule(&lp->tx_task);
> return;
> @@ -559,7 +562,7 @@ static void smc_hardware_send_pkt(unsigned long data)
>
> skb = lp->pending_tx_skb;
> if (unlikely(!skb)) {
> - smc_special_unlock(&lp->lock);
> + smc_special_unlock(&lp->lock, flags);
> return;
> }
> lp->pending_tx_skb = NULL;
> @@ -569,7 +572,7 @@ static void smc_hardware_send_pkt(unsigned long data)
> printk("%s: Memory allocation failed.\n", dev->name);
> dev->stats.tx_errors++;
> dev->stats.tx_fifo_errors++;
> - smc_special_unlock(&lp->lock);
> + smc_special_unlock(&lp->lock, flags);
> goto done;
> }
>
> @@ -608,7 +611,7 @@ static void smc_hardware_send_pkt(unsigned long data)
>
> /* queue the packet for TX */
> SMC_SET_MMU_CMD(lp, MC_ENQUEUE);
> - smc_special_unlock(&lp->lock);
> + smc_special_unlock(&lp->lock, flags);
>
> dev->trans_start = jiffies;
> dev->stats.tx_packets++;
> @@ -633,6 +636,7 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
> struct smc_local *lp = netdev_priv(dev);
> void __iomem *ioaddr = lp->base;
> unsigned int numPages, poll_count, status;
> + unsigned long flags;
>
> DBG(3, "%s: %s\n", dev->name, __func__);
>
> @@ -658,7 +662,7 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
> return 0;
> }
>
> - smc_special_lock(&lp->lock);
> + smc_special_lock(&lp->lock, flags);
>
> /* now, try to allocate the memory */
> SMC_SET_MMU_CMD(lp, MC_ALLOC | numPages);
> @@ -676,7 +680,7 @@ static int smc_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
> }
> } while (--poll_count);
>
> - smc_special_unlock(&lp->lock);
> + smc_special_unlock(&lp->lock, flags);
>
> lp->pending_tx_skb = skb;
> if (!poll_count) {
> --
> 1.6.0.4
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists