linux-kernel - RE: [PATCH] pch_gbe: Adding read memory barriers

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] [day] [month] [year] [list]

Message-ID: <1336569510.8274.360.camel@deadeye>
Date:	Wed, 9 May 2012 14:18:30 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	David Laight <David.Laight@...LAB.COM>
CC:	Erwan Velu <erwanaliasr1@...il.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <tshimizu818@...il.com>
Subject: RE: [PATCH] pch_gbe: Adding read memory barriers

On Wed, 2012-05-09 at 11:47 +0100, David Laight wrote:
> > Under a strong incoming packet stream and/or high cpu usage,
> > the pch_gbe driver reports "Receive CRC Error" and discards packet.
> > 
> > It occurred on an Intel ATOM E620T while running a 
> > 300mbit/sec multicast
> > network stream leading to a ~100% cpu usage.
> > 
> > Adding rmb() calls before considering the network card's status solve
> > this issue.
> > 
> > Getting it into stable would be perfect as it solves 
> > reliability issues.
> > 
> > Signed-off-by: Erwan Velu <erwan.velu@...iacaerospace.com>
> > ---
> >   .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |    3 +++
> >   1 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c 
> > b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> > index 8035e5f..ace3654 100644
> > --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> > +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> > @@ -1413,6 +1413,7 @@ static irqreturn_t pch_gbe_intr(int 
> > irq, void *data)
> >   			pch_gbe_mac_set_pause_packet(hw);
> >   		}
> >   	}
> > +	smp_rmb(); /* prevent any other reads before*/
> 
> Under the assumption that your memory references are uncached,
> you only need to stop gcc reordering the object code,
> Rather than actually adding one of the 'fence' instructions.

Also, the usual MMIO functions already include compiler barriers.

> So you should only need: asm volatile(:::"memory")
> NFI which define generates that, the defines in the copy of
> sysdep.h I just looked at always include one of the fences.

This is barrier().  But I think this must be intended to control
ordering of fields that are written elsewhere.

Really, this needs a much more specific comment to explain the intent.
Then reviewers can work out whether it actually achieves the intent!

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/