efx_start_datapath() asserts that we can fit 2 RX scatter buffers plus
a software structure, each cache-aligned, into a single page. Where
L1_CACHE_BYTES == 256 and PAGE_SIZE == 4096, which is the case on
s390, this assertion fails. Reduce EFX_RX_USR_BUF_SIZE to make this
work.
This should also be good for performance, as it ensures that each RX
scatter buffer covers whole cache lines and slightly reduces the use
of DMA writes that can require a read-modify-write on inter-processor
links.
(We could use 2048 - L1_CACHE_BYTES, but EFX_RX_USR_BUF_SIZE also
affects user-level networking where a larger amount of housekeeping
data may be needed. Although this version of the driver does not
support user-level networking, I prefer to keep scattering behaviour
consistent with the out-of-tree version.)
Reported-by: Geert Uytterhoeven <[email protected]>
Reported-by: Heiko Carstens <[email protected]>
Signed-off-by: Ben Hutchings <[email protected]>
---
Heiko or Geert, please confirm that this really does fix the build
failure - I don't have an s390 toolchain.
Ben.
drivers/net/ethernet/sfc/net_driver.h | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 9bd433a..6c8e1a8 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -72,8 +72,10 @@
/* Maximum possible MTU the driver supports */
#define EFX_MAX_MTU (9 * 1024)
-/* Size of an RX scatter buffer. Small enough to pack 2 into a 4K page. */
-#define EFX_RX_USR_BUF_SIZE 1824
+/* Size of an RX scatter buffer. Small enough to pack 2 into a 4K page,
+ * and should be a multiple of the cache line size.
+ */
+#define EFX_RX_USR_BUF_SIZE (2048 - 256)
/* Forward declare Precision Time Protocol (PTP) support structure. */
struct efx_ptp_data;
--
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.
Hi Ben,
On Mon, May 13, 2013 at 7:48 PM, Ben Hutchings
<[email protected]> wrote:
> efx_start_datapath() asserts that we can fit 2 RX scatter buffers plus
> a software structure, each cache-aligned, into a single page. Where
> L1_CACHE_BYTES == 256 and PAGE_SIZE == 4096, which is the case on
> s390, this assertion fails. Reduce EFX_RX_USR_BUF_SIZE to make this
> work.
>
> This should also be good for performance, as it ensures that each RX
> scatter buffer covers whole cache lines and slightly reduces the use
> of DMA writes that can require a read-modify-write on inter-processor
> links.
>
> (We could use 2048 - L1_CACHE_BYTES, but EFX_RX_USR_BUF_SIZE also
> affects user-level networking where a larger amount of housekeeping
> data may be needed. Although this version of the driver does not
> support user-level networking, I prefer to keep scattering behaviour
> consistent with the out-of-tree version.)
>
> Reported-by: Geert Uytterhoeven <[email protected]>
> Reported-by: Heiko Carstens <[email protected]>
> Signed-off-by: Ben Hutchings <[email protected]>
> ---
> Heiko or Geert, please confirm that this really does fix the build
Thanks! But unfortunately I still get the same error:
CC [M] drivers/net/ethernet/sfc/efx.o
/scratch/geert/linux/linux-m68k/drivers/net/ethernet/sfc/efx.c: In
function 'efx_start_datapath':
/scratch/geert/linux/linux-m68k/drivers/net/ethernet/sfc/efx.c:646:3:
error: call to '__compiletime_assert_648' declared with attribute
error: BUILD_BUG_ON failed: sizeof(struct efx_rx_page_state) +
EFX_PAGE_IP_ALIGN + EFX_RX_USR_BUF_SIZE > PAGE_SIZE / 2
make[4]: *** [drivers/net/ethernet/sfc/efx.o] Error 1
> failure - I don't have an s390 toolchain.
http://kernel.org/pub/tools/crosstool/files/bin/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, 2013-05-13 at 21:02 +0200, Geert Uytterhoeven wrote:
> Hi Ben,
>
> On Mon, May 13, 2013 at 7:48 PM, Ben Hutchings
> <[email protected]> wrote:
> > efx_start_datapath() asserts that we can fit 2 RX scatter buffers plus
> > a software structure, each cache-aligned, into a single page. Where
> > L1_CACHE_BYTES == 256 and PAGE_SIZE == 4096, which is the case on
> > s390, this assertion fails. Reduce EFX_RX_USR_BUF_SIZE to make this
> > work.
> >
> > This should also be good for performance, as it ensures that each RX
> > scatter buffer covers whole cache lines and slightly reduces the use
> > of DMA writes that can require a read-modify-write on inter-processor
> > links.
> >
> > (We could use 2048 - L1_CACHE_BYTES, but EFX_RX_USR_BUF_SIZE also
> > affects user-level networking where a larger amount of housekeeping
> > data may be needed. Although this version of the driver does not
> > support user-level networking, I prefer to keep scattering behaviour
> > consistent with the out-of-tree version.)
> >
> > Reported-by: Geert Uytterhoeven <[email protected]>
> > Reported-by: Heiko Carstens <[email protected]>
> > Signed-off-by: Ben Hutchings <[email protected]>
> > ---
> > Heiko or Geert, please confirm that this really does fix the build
>
> Thanks! But unfortunately I still get the same error:
>
> CC [M] drivers/net/ethernet/sfc/efx.o
> /scratch/geert/linux/linux-m68k/drivers/net/ethernet/sfc/efx.c: In
> function 'efx_start_datapath':
> /scratch/geert/linux/linux-m68k/drivers/net/ethernet/sfc/efx.c:646:3:
> error: call to '__compiletime_assert_648' declared with attribute
> error: BUILD_BUG_ON failed: sizeof(struct efx_rx_page_state) +
> EFX_PAGE_IP_ALIGN + EFX_RX_USR_BUF_SIZE > PAGE_SIZE / 2
> make[4]: *** [drivers/net/ethernet/sfc/efx.o] Error 1
OK, this doesn't work because on s390 EFX_PAGE_IP_ALIGN == 2 (this macro
is equivalent to NET_IP_ALIGN, though that wasn't always true). So DMA
is going to be misaligned on s390 anyway.
I'll see if I can work out a sensible definition that works for s390
while still being good for x86 and powerpc (which are the two we mostly
care about).
> > failure - I don't have an s390 toolchain.
>
> http://kernel.org/pub/tools/crosstool/files/bin/
I know, but this still takes time to set up.
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.
On Mon, May 13, 2013 at 9:11 PM, Ben Hutchings
<[email protected]> wrote:
>> error: BUILD_BUG_ON failed: sizeof(struct efx_rx_page_state) +
>> EFX_PAGE_IP_ALIGN + EFX_RX_USR_BUF_SIZE > PAGE_SIZE / 2
>> make[4]: *** [drivers/net/ethernet/sfc/efx.o] Error 1
>
> OK, this doesn't work because on s390 EFX_PAGE_IP_ALIGN == 2 (this macro
Yeah, that's what I just discovered, too.
#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
#define EFX_PAGE_IP_ALIGN 0
#else
#define EFX_PAGE_IP_ALIGN NET_IP_ALIGN
#endif
> is equivalent to NET_IP_ALIGN, though that wasn't always true). So DMA
> is going to be misaligned on s390 anyway.
Hmm, so it's making the choice between misaligned CPU and misaligned
DMA? Sounds fishy...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, 2013-05-13 at 21:16 +0200, Geert Uytterhoeven wrote:
> On Mon, May 13, 2013 at 9:11 PM, Ben Hutchings
> <[email protected]> wrote:
> >> error: BUILD_BUG_ON failed: sizeof(struct efx_rx_page_state) +
> >> EFX_PAGE_IP_ALIGN + EFX_RX_USR_BUF_SIZE > PAGE_SIZE / 2
> >> make[4]: *** [drivers/net/ethernet/sfc/efx.o] Error 1
> >
> > OK, this doesn't work because on s390 EFX_PAGE_IP_ALIGN == 2 (this macro
>
> Yeah, that's what I just discovered, too.
>
> #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> #define EFX_PAGE_IP_ALIGN 0
> #else
> #define EFX_PAGE_IP_ALIGN NET_IP_ALIGN
> #endif
>
> > is equivalent to NET_IP_ALIGN, though that wasn't always true). So DMA
> > is going to be misaligned on s390 anyway.
>
> Hmm, so it's making the choice between misaligned CPU and misaligned
> DMA? Sounds fishy...
When writing headers into an skb, we need to ensure that the IP header
is aligned as required by the CPU. I'm not sure whether this is true
for page buffers; it will depend on whether GRO pulls the IP header into
an skb header area before looking at it.
For RX DMA, our controller doesn't care about alignment but there is a
performance concern. So where possible we prefer to align the start of
the DMA buffer with a cache line.
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.