All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: hancockrwd@gmail.com
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-usb@vger.kernel.org
Subject: Re: [RFC PATCH] fix problems with NETIF_F_HIGHDMA in networking drivers
Date: Fri, 26 Feb 2010 01:36:37 -0800 (PST)	[thread overview]
Message-ID: <20100226.013637.255461265.davem@davemloft.net> (raw)
In-Reply-To: <4B834159.7070105@gmail.com>

From: Robert Hancock <hancockrwd@gmail.com>
Date: Mon, 22 Feb 2010 20:45:45 -0600

> Many networking drivers have issues with the use of the NETIF_F_HIGHDMA flag.
> This flag actually indicates whether or not the device/driver can handle
> skbs located in high memory (as opposed to lowmem). However, many drivers
> incorrectly treat this flag as indicating that 64-bit DMA is supported, which
> has nothing to do with its actual function. It makes no sense to make setting
> NETIF_F_HIGHDMA conditional on whether a 64-bit DMA mask has been set, as many
> drivers do, since if highmem DMA is supported at all, it should work regardless
> of whether 64-bit DMA is supported. Failing to set NETIF_F_HIGHDMA when it
> should be can hurt performance on architectures which use highmem since it
> results in needless data copying.
> 
> This fixes up the networking drivers which currently use NETIF_F_HIGHDMA to
> not do so conditionally on DMA mask settings.
> 
> For the USB kaweth and usbnet drivers, this patch also uncomments and corrects
> some code to set NETIF_F_HIGHDMA based on the USB host controller's DMA mask.
> These drivers should be able to access highmem unless the host controller is
> non-DMA-capable, which is indicated by the DMA mask being null.
> 
> Signed-off-by: Robert Hancock <hancockrwd@gmail.com>

Well, if the device isn't using 64-bit DMA addressing and the platform
uses direct (no-iommu) mapping of physical to DMA addresses , won't
your change break things?  The device will get a >4GB DMA address or
the DMA mapping layer will signal an error.

That's really part of the what the issue is I think.

So, this trigger the check in check_addr() in
arch/x86/kernel/pci-nommu.c when such packets try to get mapped by the
driver, right?

That will make the DMA mapping call fail, and the packet will be
dropped permanently.  And hey, on top of it, many of these drivers you
remove the setting from don't even check the mapping call return
values for errors.

So even bigger breakage.  One example is drivers/net/8139cp.c,
it just does dma_map_single() and uses the result.

It really depends upon that NETIF_F_HIGHDMA setting for correct
operation.

And even if something like swiotlb is available, now we're going
to do bounce buffering which is largely equivalent to what
a lack of NETIF_F_HIGHDMA will do.  Except that once NETIF_F_HIGHDMA
copies the packet to lowmem it will only do that once, whereas if
the packet goes to multiple devices swiotlb might copy the packet
to a bounce buffer multiple times.

We definitely can't apply your patch as-is.

  reply	other threads:[~2010-02-26  9:36 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-23  2:45 [RFC PATCH] fix problems with NETIF_F_HIGHDMA in networking drivers Robert Hancock
2010-02-26  9:36 ` David Miller [this message]
2010-02-26 14:46   ` Robert Hancock
2010-02-26 14:46     ` Robert Hancock
2010-02-26 15:25     ` Bartlomiej Zolnierkiewicz
2010-02-27  3:08       ` Robert Hancock
2010-02-27  3:08         ` Robert Hancock
2010-02-27  9:53         ` David Miller
2010-02-27 11:59           ` Bartlomiej Zolnierkiewicz
2010-02-27 12:05             ` David Miller
2010-02-27 18:15               ` Robert Hancock
2010-02-27 18:15                 ` Robert Hancock
2010-02-27 18:38                 ` FUJITA Tomonori
2010-02-27 18:38                   ` FUJITA Tomonori
2010-02-28  8:16                   ` David Miller
2010-02-28  8:16                     ` David Miller
2010-03-01 16:34                     ` Was: Re: [RFC PATCH] fix problems with NETIF_F_HIGHDMA in networking, Now: SWIOTLB dynamic allocation Konrad Rzeszutek Wilk
2010-03-01 21:12                       ` Robert Hancock
2010-03-01 21:12                         ` Robert Hancock
2010-03-02  4:40                         ` FUJITA Tomonori
2010-03-02  4:40                           ` FUJITA Tomonori
2010-03-02  4:40                       ` FUJITA Tomonori
2010-02-27 17:59           ` [RFC PATCH] fix problems with NETIF_F_HIGHDMA in networking drivers Robert Hancock
2010-02-27 17:59             ` Robert Hancock
2010-02-27 18:38             ` FUJITA Tomonori
2010-02-27 18:38               ` FUJITA Tomonori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100226.013637.255461265.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=hancockrwd@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.