netdev - Re: [PATCH v2] gianfar: Add I/O barriers when touching buffer descriptor ownership.

Open Source and information security mailing list archives

Date:	Wed, 2 May 2007 15:09:56 -0500
From:	Kumar Gala <galak@...nel.crashing.org>
To:	Scott Wood <scottwood@...escale.com>
Cc:	jgarzik@...ox.com, netdev@...r.kernel.org, linuxppc-dev@...abs.org
Subject: Re: [PATCH v2] gianfar: Add I/O barriers when touching buffer descriptor ownership.

On May 2, 2007, at 2:57 PM, Scott Wood wrote:

> The hardware must not see that is given ownership of a buffer until  
> it is
> completely written, and when the driver receives ownership of a  
> buffer,
> it must ensure that any other reads to the buffer reflect its final
> state.  Thus, I/O barriers are added where required.
>
> Without this patch, I have observed GCC reordering the setting of
> bdp->length and bdp->status in gfar_new_skb.
>
> Signed-off-by: Scott Wood <scottwood@...escale.com>
> ---
> This is version 2 of this patch.  I was told that eieio doesn't order
> loads from main memory, so a sync is used instead.  Also, due to
> objectons that iobarrier_* shouldn't be used for main memory, I used
> eieio() instead (a wmb() would be unnecessarily heavy).

I'd rather see a wmb() instead of eieio() to keep this code non-ppc  
specific.  (also, we implement wmb as eieio, so I don't keep the  
comment about it being too heavy, unless you mean generically).

- k

>
>  drivers/net/gianfar.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index b666a0c..b014d27 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -1025,6 +1025,7 @@ static int gfar_start_xmit(struct sk_buff  
> *skb, struct net_device *dev)
>
>  	dev->trans_start = jiffies;
>
> +	eieio();
>  	txbdp->status = status;
>
>  	/* If this was the last BD in the ring, the next one */
> @@ -1301,6 +1302,7 @@ struct sk_buff * gfar_new_skb(struct  
> net_device *dev, struct rxbd8 *bdp)
>  	bdp->length = 0;
>
>  	/* Mark the buffer empty */
> +	eieio();
>  	bdp->status |= (RXBD_EMPTY | RXBD_INTERRUPT);
>
>  	return skb;
> @@ -1484,6 +1486,7 @@ int gfar_clean_rx_ring(struct net_device  
> *dev, int rx_work_limit)
>  	bdp = priv->cur_rx;
>
>  	while (!((bdp->status & RXBD_EMPTY) || (--rx_work_limit < 0))) {
> +		rmb();
>  		skb = priv->rx_skbuff[priv->skb_currx];
>
>  		if (!(bdp->status &
> -- 
> 1.5.0.3
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@...abs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

-
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

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