From: Marek Szyprowski <m.szyprowski@samsung•com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix•de>,
netdev@vger•kernel.org
Cc: "David S. Miller" <davem@davemloft•net>,
Fabio Estevam <fabio.estevam@freescale•com>,
Frank Li <frank.li@freescale•net>,
Fugang Duan <B38611@freescale•com>,
Jim Baxter <jim_baxter@mentor•com>
Subject: Re: [PATCH] net: fec_main: dma_map() only the length of the skb
Date: Tue, 03 Dec 2013 08:36:15 +0100 [thread overview]
Message-ID: <529D89EF.1000507@samsung.com> (raw)
In-Reply-To: <1385556253-4130-1-git-send-email-bigeasy@linutronix.de>
Hello,
On 2013-11-27 13:44, Sebastian Andrzej Siewior wrote:
> On tx submit the driver always dma_map_single() FEC_ENET_TX_FRSIZE (=2048)
> bytes. This works because we don't overwrite any memory after the data buffer,
> we remove it from cache if it was there. So we hurt performace in case the
> mapping of a smaller area makes a difference.
> There is also a bug: If the data area starts shortly before the end of
> RAM say 0xc7fffa10 and the RAM ends at 0xc8000000 then we have enough
> space to fit the data area (according to skb->len) but we would map beyond
> end of ram if we are using 2048. In v2.6.31 (against which kernel this patch
> made) there is the following check in dma_cache_maint():
>
> |BUG_ON(!virt_addr_valid(start) || !virt_addr_valid(start + size - 1));
>
> Since the area starting at 0xc8000000 is no longer virt_addr_valid() we
> BUG() during dma_map_single(). The BUG() statement was removed in v3.5-rc1 as
> per 2dc6a016 ("ARM: dma-mapping: use asm-generic/dma-mapping-common.h").
>
> This patch was tested on v2.6.31 and then forward-ported and compile
> tested only against the net tree. I think it is still worth fixing
> mainline even after the BUG() statement is gone.
>
> Cc: Marek Szyprowski <m.szyprowski@samsung•com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix•de>
> ---
> It would be nice if someone could test this on current kernel.
> Is this worth pushing stable?
>
> Marek: Was there a special reason why the check was removed? Would it make
> sense to bring it back say under CONFIG_DMA_DEBUG?
I'm sorry for a delay. The check has been removed during conversion to
common,
generic dma_map_ops implementation. During those convesion
dma_(un)map_single
calls have been implemented on top of dma_(un)map_page operation, what
removed
those additional check (*_page based function didn't have such check). I
agree
that it might be a good idea to bring them back conditionally under
CONFIG_DMA_DEBUG. Would you like to send a respective patch?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
next prev parent reply other threads:[~2013-12-03 7:36 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-27 12:44 [PATCH] net: fec_main: dma_map() only the length of the skb Sebastian Andrzej Siewior
2013-11-27 13:08 ` Fugang Duan
2013-11-27 13:38 ` Sebastian Andrzej Siewior
2013-11-28 1:18 ` Fugang Duan
2013-11-28 11:24 ` Sebastian Andrzej Siewior
2013-12-02 1:25 ` David Miller
2013-12-02 2:04 ` Fugang Duan
2013-12-02 2:14 ` Fugang Duan
2013-12-02 9:52 ` [PATCH v2] " Sebastian Andrzej Siewior
2013-12-02 9:58 ` Fugang Duan
2013-12-02 21:59 ` David Miller
2013-12-03 7:36 ` Marek Szyprowski [this message]
2013-12-03 7:54 ` [PATCH] " Sebastian Andrzej Siewior
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=529D89EF.1000507@samsung.com \
--to=m.szyprowski@samsung$(echo .)com \
--cc=B38611@freescale$(echo .)com \
--cc=bigeasy@linutronix$(echo .)de \
--cc=davem@davemloft$(echo .)net \
--cc=fabio.estevam@freescale$(echo .)com \
--cc=frank.li@freescale$(echo .)net \
--cc=jim_baxter@mentor$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox