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]
Date:	Wed, 16 Jun 2010 23:20:38 -0500
From:	James Bottomley <JBottomley@...ell.com>
To:	Michael Chan <mchan@...adcom.com>
Cc:	'Mike Frysinger' <vapier@...too.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-parisc@...r.kernel.org" <linux-parisc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
Subject: Re: bnx2 fails to compile on parisc because of missing
 get_dma_ops()

On Wed, 2010-06-16 at 20:53 -0700, Michael Chan wrote:
> Mike Frysinger wrote:
> 
> > On Wed, Jun 16, 2010 at 9:13 PM, James Bottomley wrote:
> > > I'm not quite sure whose fault this one is.
> > >
> > > However, this code in bnx2.c:
> > >
> > >                if (!get_dma_ops(&pdev->dev)->sync_single_for_cpu) {
> > >                        next_rx_buf =
> > >                                &rxr->rx_buf_ring[
> > >                                        RX_RING_IDX(NEXT_RX_BD(sw_cons))];
> > >                        prefetch(next_rx_buf->desc);
> > >                }
> > >
> > > Looks remarkably fragile: what exactly is it trying to do?
> 
> If sync_single is not defined, that means the CPU has a consistent
> view of next_rx_buf and so it makes sense to prefetch it.

That's not entirely a correct statement.  Many architectures make a DMA
area coherent by turning off the CPU cache over it.  In that case,
prefetching makes absolutely no sense (although it usually works but is
a nop).

> > > The commit that causes the problem:
> > >
> > > commit a33fa66bcf365ffe5b79d1ae1d3582cc261ae56e
> > > Author: Michael Chan <mchan@...adcom.com>
> > > Date:   Thu May 6 08:58:13 2010 +0000
> > >
> > >    bnx2: Add prefetches to rx path.
> > >
> > > Looks fairly innocuous by the description.
> > >
> > > Should parisc have a get_dma_ops()?  We don't need one
> > because our dma
> > > ops are per platform not per bus.
> >
> > looks like it'll be broken on more than just parisc:
> > $ grep get_dma_ops arch/*/include/asm/ -rl | cut -d/ -f 2
> > alpha
> > ia64
> > microblaze
> > powerpc
> > sh
> > sparc
> > x86
> 
> Most of these archs use the dma functions in:
> 
> <asm-genric/dma-mapping-common.h>
> 
> so it's not a problem.

Parisc begs to differ.

Plus you're making assumptions about the contents of the ops structure
which is an internal architecture object ... that's bound to run into
portability problems even if we make it compile on all platform.

> I think I'll send in a patch to remove that part of the code
> from bnx2.c for now.

I think that's the best solution.

James


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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ