netdev - Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1295538105.2825.308.camel@edumazet-laptop>
Date:	Thu, 20 Jan 2011 16:41:45 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Po-Yu Chuang <ratbert.chuang@...il.com>
Cc:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	bhutchings@...arflare.com, joe@...ches.com, dilinger@...ued.net,
	Po-Yu Chuang <ratbert@...aday-tech.com>
Subject: Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver

Le jeudi 20 janvier 2011 à 23:30 +0800, Po-Yu Chuang a écrit :

> +static bool ftmac100_tx_complete_packet(struct ftmac100 *priv)
> +{
> +	struct net_device *netdev = priv->netdev;
> +	struct ftmac100_txdes *txdes;
> +	struct sk_buff *skb;
> +	dma_addr_t map;
> +
> +	if (priv->tx_pending == 0)
> +		return false;
> +
> +	txdes = ftmac100_current_clean_txdes(priv);
> +
> +	if (ftmac100_txdes_owned_by_dma(txdes))
> +		return false;
> +
> +	skb = ftmac100_txdes_get_skb(txdes);
> +	map = ftmac100_txdes_get_dma_addr(txdes);
> +
> +	if (unlikely(ftmac100_txdes_excessive_collision(txdes) ||
> +		     ftmac100_txdes_late_collision(txdes))) {
> +		/*
> +		 * packet transmitted to ethernet lost due to late collision
> +		 * or excessive collision
> +		 */
> +		netdev->stats.tx_aborted_errors++;
> +	} else {
> +		netdev->stats.tx_packets++;
> +		netdev->stats.tx_bytes += skb->len;
> +	}
> +
> +	dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
> +
> +	dev_kfree_skb_irq(skb);
> +
> +	ftmac100_txdes_reset(txdes);
> +
> +	ftmac100_tx_clean_pointer_advance(priv);
> +
> +	priv->tx_pending--;
> +	netif_wake_queue(netdev);
> +
> +	return true;
> +}
> +
> +static void ftmac100_tx_complete(struct ftmac100 *priv)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->tx_lock, flags);
> +	while (ftmac100_tx_complete_packet(priv))
> +		;
> +	spin_unlock_irqrestore(&priv->tx_lock, flags);
> +}
> +

I dont understand why you still block hard IRQS, after full NAPI
conversion.

Now you run from NAPI, and softirq handler, are you sure you still need
to block hard IRQ and tx_lock ?

It seems to me ftmac100_xmit() could only block softirqs (but they
already are blocked by caller), so you could use spin_lock() from
ftmac100_xmit()



--
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