* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Vlastimil Babka @ 2018-11-05 9:26 UTC (permalink / raw)
To: Aaron Lu, linux-mm, linux-kernel, netdev
Cc: Andrew Morton, Paweł Staszewski, Jesper Dangaard Brouer,
Eric Dumazet, Tariq Toukan, Ilias Apalodimas, Yoel Caspersen,
Mel Gorman, Saeed Mahameed, Michal Hocko, Dave Hansen
In-Reply-To: <20181105085820.6341-1-aaron.lu@intel.com>
On 11/5/18 9:58 AM, Aaron Lu wrote:
> page_frag_free() calls __free_pages_ok() to free the page back to
> Buddy. This is OK for high order page, but for order-0 pages, it
> misses the optimization opportunity of using Per-Cpu-Pages and can
> cause zone lock contention when called frequently.
>
> Paweł Staszewski recently shared his result of 'how Linux kernel
> handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
> found the lock contention comes from page allocator:
>
> mlx5e_poll_tx_cq
> |
> --16.34%--napi_consume_skb
> |
> |--12.65%--__free_pages_ok
> | |
> | --11.86%--free_one_page
> | |
> | |--10.10%--queued_spin_lock_slowpath
> | |
> | --0.65%--_raw_spin_lock
> |
> |--1.55%--page_frag_free
> |
> --1.44%--skb_release_data
>
> Jesper explained how it happened: mlx5 driver RX-page recycle
> mechanism is not effective in this workload and pages have to go
> through the page allocator. The lock contention happens during
> mlx5 DMA TX completion cycle. And the page allocator cannot keep
> up at these speeds.[2]
>
> I thought that __free_pages_ok() are mostly freeing high order
> pages and thought this is an lock contention for high order pages
> but Jesper explained in detail that __free_pages_ok() here are
> actually freeing order-0 pages because mlx5 is using order-0 pages
> to satisfy its page pool allocation request.[3]
>
> The free path as pointed out by Jesper is:
> skb_free_head()
> -> skb_free_frag()
> -> skb_free_frag()
> -> page_frag_free()
> And the pages being freed on this path are order-0 pages.
>
> Fix this by doing similar things as in __page_frag_cache_drain() -
> send the being freed page to PCP if it's an order-0 page, or
> directly to Buddy if it is a high order page.
>
> With this change, Paweł hasn't noticed lock contention yet in
> his workload and Jesper has noticed a 7% performance improvement
> using a micro benchmark and lock contention is gone.
>
> [1]: https://www.spinics.net/lists/netdev/msg531362.html
> [2]: https://www.spinics.net/lists/netdev/msg531421.html
> [3]: https://www.spinics.net/lists/netdev/msg531556.html
> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Yeah looks like an obvious thing to do.
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> mm/page_alloc.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ae31839874b8..91a9a6af41a2 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4555,8 +4555,14 @@ void page_frag_free(void *addr)
> {
> struct page *page = virt_to_head_page(addr);
>
> - if (unlikely(put_page_testzero(page)))
> - __free_pages_ok(page, compound_order(page));
> + if (unlikely(put_page_testzero(page))) {
> + unsigned int order = compound_order(page);
> +
> + if (order == 0)
> + free_unref_page(page);
> + else
> + __free_pages_ok(page, order);
> + }
> }
> EXPORT_SYMBOL(page_frag_free);
>
>
^ permalink raw reply
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Mel Gorman @ 2018-11-05 9:26 UTC (permalink / raw)
To: Aaron Lu
Cc: linux-mm, linux-kernel, netdev, Andrew Morton,
iso-8859-1?B?UGF3ZcWC?= Staszewski, Jesper Dangaard Brouer,
Eric Dumazet, Tariq Toukan, Ilias Apalodimas, Yoel Caspersen,
Saeed Mahameed, Michal Hocko, Vlastimil Babka, Dave Hansen
In-Reply-To: <20181105085820.6341-1-aaron.lu@intel.com>
On Mon, Nov 05, 2018 at 04:58:19PM +0800, Aaron Lu wrote:
> page_frag_free() calls __free_pages_ok() to free the page back to
> Buddy. This is OK for high order page, but for order-0 pages, it
> misses the optimization opportunity of using Per-Cpu-Pages and can
> cause zone lock contention when called frequently.
>
> [1]: https://www.spinics.net/lists/netdev/msg531362.html
> [2]: https://www.spinics.net/lists/netdev/msg531421.html
> [3]: https://www.spinics.net/lists/netdev/msg531556.html
> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Well spotted,
Acked-by: Mel Gorman <mgorman@techsingularity.net>
--
Mel Gorman
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/6] phy: Add max_bitrate attribute & phy_get_max_bitrate()
From: Marc Kleine-Budde @ 2018-11-05 9:37 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, devicetree, netdev, linux-can
Cc: wg, robh+dt, mark.rutland, kishon
In-Reply-To: <d75c9e3c-e12b-e0b8-832c-ad117dcbe990@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 1259 bytes --]
On 11/05/2018 07:27 AM, Faiz Abbas wrote:
> I remove the transceiver child node binding documentation in patch 5/6.
>
> The existing implementation is pretty limiting as it just has a child
> node with no associated device. What if a transceiver requires its own
> configurations before it can start sending/receiving messages (for
> example, my usecase requires it to pull the standby line low)?
>
> I think that can be solved by implementing the transceiver as a phy and
> exposing a generic get_max_bitrate API. That way, the transceiver device
> can do all its startup configuration in the phy probe function.
>
> In any case, do suggest if you have a better idea on how to implement
> pull gpio low requirement.
As long as we don't have any proper transceiver/phy driver, that does
more than swtich on/off a GPIO, please add a "xceiver" regulator to your
driver. Look for:
> devm_regulator_get(&pdev->dev, "xceiver");
in the flexcan driver.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Jesper Dangaard Brouer @ 2018-11-05 9:55 UTC (permalink / raw)
To: Aaron Lu
Cc: linux-mm, linux-kernel, netdev, Andrew Morton,
Paweł Staszewski, Eric Dumazet, Tariq Toukan,
Ilias Apalodimas, Yoel Caspersen, Mel Gorman, Saeed Mahameed,
Michal Hocko, Vlastimil Babka, Dave Hansen, brouer
In-Reply-To: <20181105085820.6341-1-aaron.lu@intel.com>
On Mon, 5 Nov 2018 16:58:19 +0800
Aaron Lu <aaron.lu@intel.com> wrote:
> page_frag_free() calls __free_pages_ok() to free the page back to
> Buddy. This is OK for high order page, but for order-0 pages, it
> misses the optimization opportunity of using Per-Cpu-Pages and can
> cause zone lock contention when called frequently.
>
> Paweł Staszewski recently shared his result of 'how Linux kernel
> handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
> found the lock contention comes from page allocator:
>
> mlx5e_poll_tx_cq
> |
> --16.34%--napi_consume_skb
> |
> |--12.65%--__free_pages_ok
> | |
> | --11.86%--free_one_page
> | |
> | |--10.10%--queued_spin_lock_slowpath
> | |
> | --0.65%--_raw_spin_lock
> |
> |--1.55%--page_frag_free
> |
> --1.44%--skb_release_data
>
> Jesper explained how it happened: mlx5 driver RX-page recycle
> mechanism is not effective in this workload and pages have to go
> through the page allocator. The lock contention happens during
> mlx5 DMA TX completion cycle. And the page allocator cannot keep
> up at these speeds.[2]
>
> I thought that __free_pages_ok() are mostly freeing high order
> pages and thought this is an lock contention for high order pages
> but Jesper explained in detail that __free_pages_ok() here are
> actually freeing order-0 pages because mlx5 is using order-0 pages
> to satisfy its page pool allocation request.[3]
>
> The free path as pointed out by Jesper is:
> skb_free_head()
> -> skb_free_frag()
> -> skb_free_frag()
Nitpick: you added skb_free_frag() two times, else correct.
(All this stuff gets inlined by the compiler, which makes it hard to
spot with perf report).
> -> page_frag_free()
> And the pages being freed on this path are order-0 pages.
>
> Fix this by doing similar things as in __page_frag_cache_drain() -
> send the being freed page to PCP if it's an order-0 page, or
> directly to Buddy if it is a high order page.
>
> With this change, Paweł hasn't noticed lock contention yet in
> his workload and Jesper has noticed a 7% performance improvement
> using a micro benchmark and lock contention is gone.
>
> [1]: https://www.spinics.net/lists/netdev/msg531362.html
> [2]: https://www.spinics.net/lists/netdev/msg531421.html
> [3]: https://www.spinics.net/lists/netdev/msg531556.html
> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
It is REALLY great that Aaron spotted this! (based on my analysis).
This have likely been causing scalability issues on real-life network
traffic, but have been hiding behind the driver level recycle tricks
for micro-benchmarking.
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> mm/page_alloc.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ae31839874b8..91a9a6af41a2 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4555,8 +4555,14 @@ void page_frag_free(void *addr)
> {
> struct page *page = virt_to_head_page(addr);
>
> - if (unlikely(put_page_testzero(page)))
> - __free_pages_ok(page, compound_order(page));
> + if (unlikely(put_page_testzero(page))) {
> + unsigned int order = compound_order(page);
> +
> + if (order == 0)
> + free_unref_page(page);
> + else
> + __free_pages_ok(page, order);
> + }
> }
> EXPORT_SYMBOL(page_frag_free);
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] net: phy: realtek: fix RTL8201F sysfs name
From: David Miller @ 2018-11-05 0:44 UTC (permalink / raw)
To: andrew; +Cc: holger, netdev
In-Reply-To: <20181104184741.GB27023@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Sun, 4 Nov 2018 19:47:41 +0100
> On Sun, Nov 04, 2018 at 07:02:42PM +0100, Holger Hoffstätte wrote:
>> Since 4.19 the following error in sysfs has appeared when using the
>> r8169 NIC driver:
>>
>> $cd /sys/module/realtek/drivers
>> $ls -l
>> ls: cannot access 'mdio_bus:RTL8201F 10/100Mbps Ethernet': No such file or directory
>> [..garbled dir entries follow..]
>>
>> Apparently the forward slash in "10/100Mbps Ethernet" is interpreted
>> as directory separator that leads nowhere, and was introduced in commit
>> 513588dd44b ("net: phy: realtek: add RTL8201F phy-id and functions").
>>
>> Fix this by removing the offending slash in the driver name.
>>
>> Other drivers in net/phy seem to have the same problem, but I cannot
>> test/verify them.
>
> Hi Holger
>
> This last comment would generally be placed after the ---. It will
> then not appear in the commit message.
>
> Also, in future, please put the target tree, net or net-next as part
> of the subject line:
>
> [PATCH net] ....
This didn't apply cleanly, something mangled the patch.
But I fixed it up and queued this up for -stable.
^ permalink raw reply
* Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
From: Peter Zijlstra @ 2018-11-05 10:38 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: mark.rutland@arm.com, linux-mips@linux-mips.org,
will.deacon@arm.com, bfields@fieldses.org, paulus@samba.org,
Trond Myklebust, jhogan@kernel.org, Paul McKenney,
linux@roeck-us.net, arnd@arndb.de, boqun.feng@gmail.com, dvyukov,
linux-nfs@vger.kernel.org, netdev@vger.kernel.org,
jlayton@kernel.org, linux-kernel@vger.kernel.org,
ralf@linux-mips.org, anna.schumaker@netapp.com,
paul.burton@mips.com, "akpm@linu
In-Reply-To: <5a846924-e642-d9d1-4e0e-810bd4d01c26@virtuozzo.com>
On Fri, Nov 02, 2018 at 07:19:15PM +0300, Andrey Ryabinin wrote:
> UBSAN warns about signed overflows despite -fno-strict-overflow if gcc
> version is < 8. I have learned recently that UBSAN in GCC 8 ignores
> signed overflows if -fno-strict-overflow of fwrapv is used.
Ah, good.
> We can always just drop -fsanitize=signed-integer-overflow if it considered too noisy.
I think that is the most consistent beahviour. signed overflow is not UB
in the kernel.
> Although it did catch some real bugs.
If we want an over/under-flow checker, then that should be a separate
plugin and not specific to signed or unsigned.
^ permalink raw reply
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Ilias Apalodimas @ 2018-11-05 10:46 UTC (permalink / raw)
To: Aaron Lu
Cc: linux-mm, linux-kernel, netdev, Andrew Morton,
Paweł Staszewski, Jesper Dangaard Brouer, Eric Dumazet,
Tariq Toukan, Yoel Caspersen, Mel Gorman, Saeed Mahameed,
Michal Hocko, Vlastimil Babka, Dave Hansen
In-Reply-To: <20181105085820.6341-1-aaron.lu@intel.com>
Hi Aaron,
> page_frag_free() calls __free_pages_ok() to free the page back to
> Buddy. This is OK for high order page, but for order-0 pages, it
> misses the optimization opportunity of using Per-Cpu-Pages and can
> cause zone lock contention when called frequently.
>
> Paweł Staszewski recently shared his result of 'how Linux kernel
> handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
> found the lock contention comes from page allocator:
>
> mlx5e_poll_tx_cq
> |
> --16.34%--napi_consume_skb
> |
> |--12.65%--__free_pages_ok
> | |
> | --11.86%--free_one_page
> | |
> | |--10.10%--queued_spin_lock_slowpath
> | |
> | --0.65%--_raw_spin_lock
> |
> |--1.55%--page_frag_free
> |
> --1.44%--skb_release_data
>
> Jesper explained how it happened: mlx5 driver RX-page recycle
> mechanism is not effective in this workload and pages have to go
> through the page allocator. The lock contention happens during
> mlx5 DMA TX completion cycle. And the page allocator cannot keep
> up at these speeds.[2]
>
> I thought that __free_pages_ok() are mostly freeing high order
> pages and thought this is an lock contention for high order pages
> but Jesper explained in detail that __free_pages_ok() here are
> actually freeing order-0 pages because mlx5 is using order-0 pages
> to satisfy its page pool allocation request.[3]
>
> The free path as pointed out by Jesper is:
> skb_free_head()
> -> skb_free_frag()
> -> skb_free_frag()
> -> page_frag_free()
> And the pages being freed on this path are order-0 pages.
>
> Fix this by doing similar things as in __page_frag_cache_drain() -
> send the being freed page to PCP if it's an order-0 page, or
> directly to Buddy if it is a high order page.
>
> With this change, Paweł hasn't noticed lock contention yet in
> his workload and Jesper has noticed a 7% performance improvement
> using a micro benchmark and lock contention is gone.
I did the same tests on a 'low' speed 1Gbit interface on an cortex-a53.
I used socionext's netsec driver and switched buffer allocation from the
current scheme to using page_pool API (which by default allocates order0
pages).
Running 'perf top' pre and post patch got me the same results.
__free_pages_ok() disappeared from perf top and i got an ~11%
performance boost testing with 64byte packets.
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Tested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply
* Re: [PATCH][ath10k-next] ath10k: fix some spelling mistakes
From: Kalle Valo @ 2018-11-05 11:12 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, ath10k, linux-wireless, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20181018215104.10444-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> Trivial fix to some spelling mistakes in ath10k_err and ath10k_dbg
> messages:
> "capablity" -> "capability"
> "registed" -> "registered"
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
eb9bd8b963d6 ath10k: fix some spelling mistakes
--
https://patchwork.kernel.org/patch/10648317/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH][ath10k-next] ath10k: fix some spelling mistakes
From: Kalle Valo @ 2018-11-05 11:12 UTC (permalink / raw)
To: Colin King
Cc: linux-wireless, netdev, kernel-janitors, linux-kernel, ath10k,
David S . Miller
In-Reply-To: <20181018215104.10444-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> Trivial fix to some spelling mistakes in ath10k_err and ath10k_dbg
> messages:
> "capablity" -> "capability"
> "registed" -> "registered"
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
eb9bd8b963d6 ath10k: fix some spelling mistakes
--
https://patchwork.kernel.org/patch/10648317/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/6] phy: Add max_bitrate attribute & phy_get_max_bitrate()
From: Faiz Abbas @ 2018-11-05 11:14 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-kernel, devicetree, netdev, linux-can
Cc: wg, robh+dt, mark.rutland, kishon
In-Reply-To: <0c0885c4-f5ae-a29c-7176-f67fe546cfd8@pengutronix.de>
Hi,
On Monday 05 November 2018 03:07 PM, Marc Kleine-Budde wrote:
> On 11/05/2018 07:27 AM, Faiz Abbas wrote:
>> I remove the transceiver child node binding documentation in patch 5/6.
>>
>> The existing implementation is pretty limiting as it just has a child
>> node with no associated device. What if a transceiver requires its own
>> configurations before it can start sending/receiving messages (for
>> example, my usecase requires it to pull the standby line low)?
>>
>> I think that can be solved by implementing the transceiver as a phy and
>> exposing a generic get_max_bitrate API. That way, the transceiver device
>> can do all its startup configuration in the phy probe function.
>>
>> In any case, do suggest if you have a better idea on how to implement
>> pull gpio low requirement.
>
> As long as we don't have any proper transceiver/phy driver, that does
> more than swtich on/off a GPIO, please add a "xceiver" regulator to your
> driver. Look for:
>
>> devm_regulator_get(&pdev->dev, "xceiver");
>
The transceiver is not specific to m_can. The pull down would be
required even if it were connected to some other controller.
Thanks,
Faiz
^ permalink raw reply
* Re: [PATCH 02/20] ath9k: ar5008_phy: mark expected switch fall-through
From: Kalle Valo @ 2018-11-05 11:16 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: QCA ath9k Development, linux-wireless, David S. Miller, netdev,
linux-kernel, Gustavo A. R. Silva
In-Reply-To: <1727f5c9f0bb87651831d267e35af5aa1ed1f98b.1540239684.git.gustavo@embeddedor.com>
"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 1056532 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
3 patches applied to ath-next branch of ath.git, thanks.
604e98670072 ath9k: ar5008_phy: mark expected switch fall-through
f2a881055094 ath9k: ar9002_phy: mark expected switch fall-throughs
bf74fd75137d ath9k: hw: mark expected switch fall-through
--
https://patchwork.kernel.org/patch/10652551/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] ath9k: remove set but not used variable 'new_flags'
From: Kalle Valo @ 2018-11-05 11:18 UTC (permalink / raw)
To: zhong jiang; +Cc: davem, linux-wireless, netdev, ath9k-devel, linux-kernel
In-Reply-To: <1540265234-58050-1-git-send-email-zhongjiang@huawei.com>
zhong jiang <zhongjiang@huawei.com> wrote:
> new_flags is not used after setting its value. It is safe to remove
> the unused variable.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
53d76f1687ba ath9k: remove set but not used variable 'new_flags'
--
https://patchwork.kernel.org/patch/10652679/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 01/20] ath6kl: Mark expected switch fall-through
From: Kalle Valo @ 2018-11-05 11:23 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: QCA ath9k Development, linux-wireless, David S. Miller, netdev,
linux-kernel, Gustavo A. R. Silva
In-Reply-To: <7932c27ae4e53aceacb7a0a1ae26db91432e5014.1540239684.git.gustavo@embeddedor.com>
"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 201383 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Acked-by: Steve deRosier <derosier@cal-sierra.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
3 patches applied to ath-next branch of ath.git, thanks.
6d9e1ecd1537 ath6kl: Mark expected switch fall-through
003161445bc8 carl9170: rx: mark expected switch fall-through
f458f9f43690 carl9170: tx: mark expected switch fall-throughs
--
https://patchwork.kernel.org/patch/10652473/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* RE: [PATCH] net/mlx5e: fix high stack usage
From: David Laight @ 2018-11-05 11:24 UTC (permalink / raw)
To: 'Arnd Bergmann', Saeed Mahameed, Leon Romanovsky,
David S. Miller
Cc: Tariq Toukan, Eran Ben Elisha, Boris Pismenny, Ilya Lesokhin,
Moshe Shemesh, Kamal Heib, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20181102153316.1492515-1-arnd@arndb.de>
From: Arnd Bergmann
> Sent: 02 November 2018 15:33
>
> A patch that looks harmless causes the stack usage of the mlx5e_grp_sw_update_stats()
> function to drastically increase with x86 gcc-4.9 and higher (tested up to 8.1):
>
> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c: In function ‘mlx5e_grp_sw_update_stats’:
> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c:216:1: warning: the frame size of 1276 bytes is
> larger than 500 bytes [-Wframe-larger-than=]
>
> By splitting out the loop body into a non-inlined function, the stack size goes
> back down to under 500 bytes.
I'd look at the generated code for the function.
It might be truly horrid.
I suspect that gcc allocates 'virtual registers' for all the
s->tx_... members and then writes them to 's' outside the loop.
Unfortunately there aren't enough real registers so all the
virtual ones get spilled to stack.
I've seen it do something similar (extra spills to stack) in the
ixgbe byte and packet counting.
I think it is really a gcc bug.
...
> + for (j = 0; j < priv->max_opened_tc; j++) {
> + struct mlx5e_sq_stats *sq_stats = &channel_stats->sq[j];
Try adding barrier() here.
> +
> + s->tx_packets += sq_stats->packets;
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH 1/6] phy: Add max_bitrate attribute & phy_get_max_bitrate()
From: Marc Kleine-Budde @ 2018-11-05 11:47 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, devicetree, netdev, linux-can
Cc: wg, robh+dt, mark.rutland, kishon
In-Reply-To: <5685cb49-a6bf-3696-f984-020847633e5e@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 2320 bytes --]
On 11/05/2018 12:14 PM, Faiz Abbas wrote:
> Hi,
>
> On Monday 05 November 2018 03:07 PM, Marc Kleine-Budde wrote:
>> On 11/05/2018 07:27 AM, Faiz Abbas wrote:
>>> I remove the transceiver child node binding documentation in patch 5/6.
>>>
>>> The existing implementation is pretty limiting as it just has a child
>>> node with no associated device. What if a transceiver requires its own
>>> configurations before it can start sending/receiving messages (for
>>> example, my usecase requires it to pull the standby line low)?
>>>
>>> I think that can be solved by implementing the transceiver as a phy and
>>> exposing a generic get_max_bitrate API. That way, the transceiver device
>>> can do all its startup configuration in the phy probe function.
>>>
>>> In any case, do suggest if you have a better idea on how to implement
>>> pull gpio low requirement.
>>
>> As long as we don't have any proper transceiver/phy driver, that does
>> more than swtich on/off a GPIO, please add a "xceiver" regulator to your
>> driver. Look for:
>>
>>> devm_regulator_get(&pdev->dev, "xceiver");
>>
>
> The transceiver is not specific to m_can. The pull down would be
> required even if it were connected to some other controller.
Ok, this is a quite common pattern. For the fsl/nxp boards we add the
regulator to the board dts. See "imx28-evk.dts" for example:
> can0: can@80032000 {
> pinctrl-names = "default";
> pinctrl-0 = <&can0_pins_a>;
> xceiver-supply = <®_can_3v3>;
> status = "okay";
> };
>
> can1: can@80034000 {
> pinctrl-names = "default";
> pinctrl-0 = <&can1_pins_a>;
> xceiver-supply = <®_can_3v3>;
> status = "okay";
> };
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: Grant
From: M. M. Fridman @ 2018-10-13 7:07 UTC (permalink / raw)
To: Recipients
I, Mikhail Fridman have selected you specifically as one of my beneficiaries for my Charitable Donation of $5 Million Dollars,
Check the link below for confirmation:
https://www.rt.com/business/343781-mikhail-fridman-will-charity/
I await your earliest response for further directives.
Best Regards,
Mikhail Fridman.
^ permalink raw reply
* Re: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
From: Ivan Labáth @ 2018-11-05 13:06 UTC (permalink / raw)
To: Jason A. Donenfeld, Dave Taht
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman
In-Reply-To: <CAHmME9oQJ9y51Pv7U=+=5uDA_tb+U1zWZdsLcyNMhdcFZmjM8Q@mail.gmail.com>
On 26. 9. 2018 18:04, Jason A. Donenfeld wrote:
> Hi Ivan,
>
> On Wed, Sep 26, 2018 at 6:00 PM Ivan Labáth <labokml@labo.rs> wrote:
>>
>> On 25.09.2018 16:56, Jason A. Donenfeld wrote:
>>> Extensive documentation and description of the protocol and
>>> considerations, along with formal proofs of the cryptography, are> available at:
>>>
>>> * https://www.wireguard.com/
>>> * https://www.wireguard.com/papers/wireguard.pdf
>> []
>>> +enum { HANDSHAKE_DSCP = 0x88 /* AF41, plus 00 ECN */ };
>> []
>>> + if (skb->protocol == htons(ETH_P_IP)) {
>>> + len = ntohs(ip_hdr(skb)->tot_len);
>>> + if (unlikely(len < sizeof(struct iphdr)))
>>> + goto dishonest_packet_size;
>>> + if (INET_ECN_is_ce(PACKET_CB(skb)->ds))
>>> + IP_ECN_set_ce(ip_hdr(skb));
>>> + } else if (skb->protocol == htons(ETH_P_IPV6)) {
>>> + len = ntohs(ipv6_hdr(skb)->payload_len) +
>>> + sizeof(struct ipv6hdr);
>>> + if (INET_ECN_is_ce(PACKET_CB(skb)->ds))
>>> + IP6_ECN_set_ce(skb, ipv6_hdr(skb));
>>> + } else
>> []
>>> + skb_queue_walk (&packets, skb) {
>>> + /* 0 for no outer TOS: no leak. TODO: should we use flowi->tos
>>> + * as outer? */
>>> + PACKET_CB(skb)->ds = ip_tunnel_ecn_encap(0, ip_hdr(skb), skb);
>>> + PACKET_CB(skb)->nonce =
>>> + atomic64_inc_return(&key->counter.counter) - 1;
>>> + if (unlikely(PACKET_CB(skb)->nonce >= REJECT_AFTER_MESSAGES))
>>> + goto out_invalid;
>>> + }
>> Hi,
>>
>> is there documentation and/or rationale for ecn handling?
>> Quick search for ecn and dscp didn't reveal any.
>
> ECN support was developed with Dave Taht so that it does the right
> thing with CAKE and such. He's CC'd, so that he can fill in details,
> and sure, we can write these up. As well, I can add the rationale for
> the handshake-packet-specific DSCP value to the paper in the next few
> days; thanks for pointing out these documentation oversights.
>
> Jason
>
Any news on this?
To be clear, question is not about an insignificant documentation
oversight. It is about copying bits from inner packets to outer packets
of a secure* tunnel and documenting it AFAICT nowhere, while claiming
extensive documentation.
* it really should be specified what secure tunnel means, as it has many
plausible interpretations and wireguard surely does not fulfill all of them.
Ivan
^ permalink raw reply
* Re: [PATCH] net: skbuff.h: remove unnecessary unlikely()
From: Frank Lee @ 2018-11-05 13:21 UTC (permalink / raw)
To: davem, Eric Dumazet, Willem de Bruijn, dja, ast, sbrivio, pabeni
Cc: linux-kernel, netdev
In-Reply-To: <20181104131444.4049-1-tiny.windzz@gmail.com>
add netdev@vger.kernel.org
-- Yangtao
On Sun, Nov 4, 2018 at 9:14 PM Yangtao Li <tiny.windzz@gmail.com> wrote:
>
> WARN_ON() already contains an unlikely(), so it's not necessary to use
> unlikely.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> include/linux/skbuff.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 0ba687454267..7dcfb5591dc3 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2508,10 +2508,8 @@ int ___pskb_trim(struct sk_buff *skb, unsigned int len);
>
> static inline void __skb_set_length(struct sk_buff *skb, unsigned int len)
> {
> - if (unlikely(skb_is_nonlinear(skb))) {
> - WARN_ON(1);
> + if (WARN_ON(skb_is_nonlinear(skb)))
> return;
> - }
> skb->len = len;
> skb_set_tail_pointer(skb, len);
> }
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
From: Suraj Singh @ 2018-11-05 13:53 UTC (permalink / raw)
To: edumazet; +Cc: davem, kuznet, yoshfuji, netdev, linux-kernel, Suraj Singh
Fixed warnings and checks for TCP Westwood
Signed-off-by: Suraj Singh <suraj1998@gmail.com>
---
net/ipv4/tcp_westwood.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/net/ipv4/tcp_westwood.c b/net/ipv4/tcp_westwood.c
index bec9caf..0b4c67a9 100644
--- a/net/ipv4/tcp_westwood.c
+++ b/net/ipv4/tcp_westwood.c
@@ -43,11 +43,10 @@ struct westwood {
};
/* TCP Westwood functions and constants */
-#define TCP_WESTWOOD_RTT_MIN (HZ/20) /* 50ms */
-#define TCP_WESTWOOD_INIT_RTT (20*HZ) /* maybe too conservative?! */
+#define TCP_WESTWOOD_RTT_MIN (HZ / 20) /* 50ms */
+#define TCP_WESTWOOD_INIT_RTT (20 * HZ) /* maybe too conservative?! */
-/*
- * @tcp_westwood_create
+/* @tcp_westwood_create
* This function initializes fields used in TCP Westwood+,
* it is called after the initial SYN, so the sequence numbers
* are correct but new passive connections we have no
@@ -67,14 +66,14 @@ static void tcp_westwood_init(struct sock *sk)
w->accounted = 0;
w->cumul_ack = 0;
w->reset_rtt_min = 1;
- w->rtt_min = w->rtt = TCP_WESTWOOD_INIT_RTT;
+ w->rtt_min = TCP_WESTWOOD_INIT_RTT;
+ w->rtt = TCP_WESTWOOD_INIT_RTT;
w->rtt_win_sx = tcp_jiffies32;
w->snd_una = tcp_sk(sk)->snd_una;
w->first_ack = 1;
}
-/*
- * @westwood_do_filter
+/* @westwood_do_filter
* Low-pass filter. Implemented using constant coefficients.
*/
static inline u32 westwood_do_filter(u32 a, u32 b)
@@ -94,8 +93,7 @@ static void westwood_filter(struct westwood *w, u32 delta)
}
}
-/*
- * @westwood_pkts_acked
+/* @westwood_pkts_acked
* Called after processing group of packets.
* but all westwood needs is the last sample of srtt.
*/
@@ -108,8 +106,7 @@ static void tcp_westwood_pkts_acked(struct sock *sk,
w->rtt = usecs_to_jiffies(sample->rtt_us);
}
-/*
- * @westwood_update_window
+/* @westwood_update_window
* It updates RTT evaluation window if it is the right moment to do
* it. If so it calls filter for evaluating bandwidth.
*/
@@ -127,8 +124,7 @@ static void westwood_update_window(struct sock *sk)
w->first_ack = 0;
}
- /*
- * See if a RTT-window has passed.
+ /* See if a RTT-window has passed.
* Be careful since if RTT is less than
* 50ms we don't filter but we continue 'building the sample'.
* This minimum limit was chosen since an estimation on small
@@ -149,12 +145,12 @@ static inline void update_rtt_min(struct westwood *w)
if (w->reset_rtt_min) {
w->rtt_min = w->rtt;
w->reset_rtt_min = 0;
- } else
+ } else {
w->rtt_min = min(w->rtt, w->rtt_min);
+ }
}
-/*
- * @westwood_fast_bw
+/* @westwood_fast_bw
* It is called when we are in fast path. In particular it is called when
* header prediction is successful. In such case in fact update is
* straight forward and doesn't need any particular care.
@@ -171,8 +167,7 @@ static inline void westwood_fast_bw(struct sock *sk)
update_rtt_min(w);
}
-/*
- * @westwood_acked_count
+/* @westwood_acked_count
* This function evaluates cumul_ack for evaluating bk in case of
* delayed or partial acks.
*/
@@ -207,8 +202,7 @@ static inline u32 westwood_acked_count(struct sock *sk)
return w->cumul_ack;
}
-/*
- * TCP Westwood
+/* TCP Westwood
* Here limit is evaluated as Bw estimation*RTTmin (for obtaining it
* in packets we use mss_cache). Rttmin is guaranteed to be >= 2
* so avoids ever returning 0.
@@ -243,7 +237,8 @@ static void tcp_westwood_event(struct sock *sk, enum tcp_ca_event event)
switch (event) {
case CA_EVENT_COMPLETE_CWR:
- tp->snd_cwnd = tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
+ tp->snd_cwnd = tcp_westwood_bw_rttmin(sk);
+ tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
break;
case CA_EVENT_LOSS:
tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
From: Peter Zijlstra @ 2018-11-05 14:24 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Trond Myklebust, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, ralf@linux-mips.org,
jlayton@kernel.org, linuxppc-dev@lists.ozlabs.org,
bfields@fieldses.org, linux-mips@linux-mips.org,
linux@roeck-us.net, linux-nfs@vger.kernel.org,
akpm@linux-foundation.org, will.deacon@arm.com,
boqun.feng@gmail.com, paul.burton@mips.com,
anna.schumaker@netapp.com, "jhogan@kerne
In-Reply-To: <5a846924-e642-d9d1-4e0e-810bd4d01c26@virtuozzo.com>
On Fri, Nov 02, 2018 at 07:19:15PM +0300, Andrey Ryabinin wrote:
> UBSAN warns about signed overflows despite -fno-strict-overflow if gcc version is < 8.
> I have learned recently that UBSAN in GCC 8 ignores signed overflows if -fno-strict-overflow of fwrapv is used.
>
> $ cat signed_overflow.c
> #include <stdio.h>
>
> __attribute__((noinline))
> int foo(int a, int b)
> {
> return a+b;
s/+/<</
> }
>
> int main(void)
> {
> int a = 0x7fffffff;
> int b = 2;
> printf("%d\n", foo(a,b));
> return 0;
> }
It also seem to affect 'shift':
peterz@hirez:~/tmp$ gcc -fsanitize=signed-integer-overflow,shift overflow.c ; ./a.out
overflow.c:6:11: runtime error: left shift of 2147483647 by 2 places cannot be represented in type 'int'
-4
peterz@hirez:~/tmp$ gcc -fsanitize=signed-integer-overflow,shift -fwrapv overflow.c ; ./a.out
-4
^ permalink raw reply
* [PATCH] net/bluetooth: Change to use DEFINE_SHOW_ATTRIBUTE macro
From: Yangtao Li @ 2018-11-05 14:56 UTC (permalink / raw)
To: marcel, johan.hedberg, davem, keescook, gustavo, dvlasenk
Cc: linux-bluetooth, netdev, linux-kernel, Yangtao Li
Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
net/bluetooth/l2cap_core.c | 12 +-----------
net/bluetooth/rfcomm/core.c | 12 +-----------
net/bluetooth/rfcomm/sock.c | 12 +-----------
net/bluetooth/sco.c | 12 +-----------
4 files changed, 4 insertions(+), 44 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d17a4736e47c..38d92ced5fb5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7615,17 +7615,7 @@ static int l2cap_debugfs_show(struct seq_file *f, void *p)
return 0;
}
-static int l2cap_debugfs_open(struct inode *inode, struct file *file)
-{
- return single_open(file, l2cap_debugfs_show, inode->i_private);
-}
-
-static const struct file_operations l2cap_debugfs_fops = {
- .open = l2cap_debugfs_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(l2cap_debugfs);
static struct dentry *l2cap_debugfs;
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index b98225d65e87..1a635df80643 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2166,17 +2166,7 @@ static int rfcomm_dlc_debugfs_show(struct seq_file *f, void *x)
return 0;
}
-static int rfcomm_dlc_debugfs_open(struct inode *inode, struct file *file)
-{
- return single_open(file, rfcomm_dlc_debugfs_show, inode->i_private);
-}
-
-static const struct file_operations rfcomm_dlc_debugfs_fops = {
- .open = rfcomm_dlc_debugfs_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(rfcomm_dlc_debugfs);
static struct dentry *rfcomm_dlc_debugfs;
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index d606e9212291..aa0db1d1bd9b 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -1020,17 +1020,7 @@ static int rfcomm_sock_debugfs_show(struct seq_file *f, void *p)
return 0;
}
-static int rfcomm_sock_debugfs_open(struct inode *inode, struct file *file)
-{
- return single_open(file, rfcomm_sock_debugfs_show, inode->i_private);
-}
-
-static const struct file_operations rfcomm_sock_debugfs_fops = {
- .open = rfcomm_sock_debugfs_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(rfcomm_sock_debugfs);
static struct dentry *rfcomm_sock_debugfs;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 8f0f9279eac9..529b38996d8b 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -1173,17 +1173,7 @@ static int sco_debugfs_show(struct seq_file *f, void *p)
return 0;
}
-static int sco_debugfs_open(struct inode *inode, struct file *file)
-{
- return single_open(file, sco_debugfs_show, inode->i_private);
-}
-
-static const struct file_operations sco_debugfs_fops = {
- .open = sco_debugfs_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(sco_debugfs);
static struct dentry *sco_debugfs;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Alexander Duyck @ 2018-11-05 15:44 UTC (permalink / raw)
To: aaron.lu
Cc: linux-mm, LKML, Netdev, Andrew Morton, Paweł Staszewski,
Jesper Dangaard Brouer, Eric Dumazet, Tariq Toukan,
ilias.apalodimas, yoel, Mel Gorman, Saeed Mahameed, Michal Hocko,
Vlastimil Babka, dave.hansen
In-Reply-To: <20181105085820.6341-1-aaron.lu@intel.com>
On Mon, Nov 5, 2018 at 12:58 AM Aaron Lu <aaron.lu@intel.com> wrote:
>
> page_frag_free() calls __free_pages_ok() to free the page back to
> Buddy. This is OK for high order page, but for order-0 pages, it
> misses the optimization opportunity of using Per-Cpu-Pages and can
> cause zone lock contention when called frequently.
>
> Paweł Staszewski recently shared his result of 'how Linux kernel
> handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
> found the lock contention comes from page allocator:
>
> mlx5e_poll_tx_cq
> |
> --16.34%--napi_consume_skb
> |
> |--12.65%--__free_pages_ok
> | |
> | --11.86%--free_one_page
> | |
> | |--10.10%--queued_spin_lock_slowpath
> | |
> | --0.65%--_raw_spin_lock
> |
> |--1.55%--page_frag_free
> |
> --1.44%--skb_release_data
>
> Jesper explained how it happened: mlx5 driver RX-page recycle
> mechanism is not effective in this workload and pages have to go
> through the page allocator. The lock contention happens during
> mlx5 DMA TX completion cycle. And the page allocator cannot keep
> up at these speeds.[2]
>
> I thought that __free_pages_ok() are mostly freeing high order
> pages and thought this is an lock contention for high order pages
> but Jesper explained in detail that __free_pages_ok() here are
> actually freeing order-0 pages because mlx5 is using order-0 pages
> to satisfy its page pool allocation request.[3]
>
> The free path as pointed out by Jesper is:
> skb_free_head()
> -> skb_free_frag()
> -> skb_free_frag()
> -> page_frag_free()
> And the pages being freed on this path are order-0 pages.
>
> Fix this by doing similar things as in __page_frag_cache_drain() -
> send the being freed page to PCP if it's an order-0 page, or
> directly to Buddy if it is a high order page.
>
> With this change, Paweł hasn't noticed lock contention yet in
> his workload and Jesper has noticed a 7% performance improvement
> using a micro benchmark and lock contention is gone.
>
> [1]: https://www.spinics.net/lists/netdev/msg531362.html
> [2]: https://www.spinics.net/lists/netdev/msg531421.html
> [3]: https://www.spinics.net/lists/netdev/msg531556.html
> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
> mm/page_alloc.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ae31839874b8..91a9a6af41a2 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4555,8 +4555,14 @@ void page_frag_free(void *addr)
> {
> struct page *page = virt_to_head_page(addr);
>
> - if (unlikely(put_page_testzero(page)))
> - __free_pages_ok(page, compound_order(page));
> + if (unlikely(put_page_testzero(page))) {
> + unsigned int order = compound_order(page);
> +
> + if (order == 0)
> + free_unref_page(page);
> + else
> + __free_pages_ok(page, order);
> + }
> }
> EXPORT_SYMBOL(page_frag_free);
>
One thing I would suggest for Pawel to try would be to reduce the Tx
qdisc size on his transmitting interfaces, Reduce the Tx ring size,
and possibly increase the Tx interrupt rate. Ideally we shouldn't have
too many packets in-flight and I suspect that is the issue that Pawel
is seeing that is leading to the page pool allocator freeing up the
memory. I know we like to try to batch things but the issue is
processing too many Tx buffers in one batch leads to us eating up too
much memory and causing evictions from the cache. Ideally the Rx and
Tx rings and queues should be sized as small as possible while still
allowing us to process up to our NAPI budget. Usually I run things
with a 128 Rx / 128 Tx setup and then reduce the Tx queue length so we
don't have more buffers stored there than we can place in the Tx ring.
Then we can avoid the extra thrash of having to pull/push memory into
and out of the freelists. Essentially the issue here ends up being
another form of buffer bloat.
With that said this change should be mostly harmless and does address
the fact that we can have both regular order 0 pages and page frags
used for skb->head.
Acked-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Aaron Lu @ 2018-11-05 6:28 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Saeed Mahameed, pstaszewski@itcare.pl, eric.dumazet@gmail.com,
netdev@vger.kernel.org, Tariq Toukan, ilias.apalodimas@linaro.org,
yoel@kviknet.dk, mgorman@techsingularity.net
In-Reply-To: <20181103135325.01a7b5d6@redhat.com>
On Sat, Nov 03, 2018 at 01:53:25PM +0100, Jesper Dangaard Brouer wrote:
>
> On Fri, 2 Nov 2018 22:20:24 +0800 Aaron Lu <aaron.lu@intel.com> wrote:
>
> > On Fri, Nov 02, 2018 at 12:40:37PM +0100, Jesper Dangaard Brouer wrote:
> > > On Fri, 2 Nov 2018 13:23:56 +0800
> > > Aaron Lu <aaron.lu@intel.com> wrote:
> > >
> > > > On Thu, Nov 01, 2018 at 08:23:19PM +0000, Saeed Mahameed wrote:
> > > > > On Thu, 2018-11-01 at 23:27 +0800, Aaron Lu wrote:
> > > > > > On Thu, Nov 01, 2018 at 10:22:13AM +0100, Jesper Dangaard Brouer
> > > > > > wrote:
> > > > > > ... ...
> > > > > > > Section copied out:
> > > > > > >
> > > > > > > mlx5e_poll_tx_cq
> > > > > > > |
> > > > > > > --16.34%--napi_consume_skb
> > > > > > > |
> > > > > > > |--12.65%--__free_pages_ok
> > > > > > > | |
> > > > > > > | --11.86%--free_one_page
> > > > > > > | |
> > > > > > > | |--10.10%
> > > > > > > --queued_spin_lock_slowpath
> > > > > > > | |
> > > > > > > | --0.65%--_raw_spin_lock
> > > > > >
> > > > > > This callchain looks like it is freeing higher order pages than order
> > > > > > 0:
> > > > > > __free_pages_ok is only called for pages whose order are bigger than
> > > > > > 0.
> > > > >
> > > > > mlx5 rx uses only order 0 pages, so i don't know where these high order
> > > > > tx SKBs are coming from..
> > > >
> > > > Perhaps here:
> > > > __netdev_alloc_skb(), __napi_alloc_skb(), __netdev_alloc_frag() and
> > > > __napi_alloc_frag() will all call page_frag_alloc(), which will use
> > > > __page_frag_cache_refill() to get an order 3 page if possible, or fall
> > > > back to an order 0 page if order 3 page is not available.
> > > >
> > > > I'm not sure if your workload will use the above code path though.
> > >
> > > TL;DR: this is order-0 pages (code-walk trough proof below)
> > >
> > > To Aaron, the network stack *can* call __free_pages_ok() with order-0
> > > pages, via:
> > >
> > > static void skb_free_head(struct sk_buff *skb)
> > > {
> > > unsigned char *head = skb->head;
> > >
> > > if (skb->head_frag)
> > > skb_free_frag(head);
> > > else
> > > kfree(head);
> > > }
> > >
> > > static inline void skb_free_frag(void *addr)
> > > {
> > > page_frag_free(addr);
> > > }
> > >
> > > /*
> > > * Frees a page fragment allocated out of either a compound or order 0 page.
> > > */
> > > void page_frag_free(void *addr)
> > > {
> > > struct page *page = virt_to_head_page(addr);
> > >
> > > if (unlikely(put_page_testzero(page)))
> > > __free_pages_ok(page, compound_order(page));
> > > }
> > > EXPORT_SYMBOL(page_frag_free);
> >
> > I think here is a problem - order 0 pages are freed directly to buddy,
> > bypassing per-cpu-pages. This might be the reason lock contention
> > appeared on free path.
>
> OMG - you just found a significant issue with the network stacks
> interaction with the page allocator! This explains why I could not get
> the PCP (Per-Cpu-Pages) system to have good performance, in my
> performance networking benchmarks. As we are basically only using the
> alloc side of PCP, and not the free side.
Exactly.
> We have spend years adding different driver level recycle tricks to
> avoid this code path getting activated, exactly because it is rather
> slow and problematic that we hit this zone->lock.
I can see when this code path is hit, it causes unnecessary taking of
zone lock for order-0 pages and cause lock contention.
>
> > Can someone apply below diff and see if lock contention is gone?
>
> I have also applied and tested this patch, and yes the lock contention
> is gone. As mentioned is it rather difficult to hit this code path, as
> the driver page recycle mechanism tries to hide/avoid it, but mlx5 +
> page_pool + CPU-map recycling have a known weakness that bypass the
> driver page recycle scheme (that I've not fixed yet). I observed a 7%
> speedup for this micro benchmark.
Good to know this, I will prepare a formal patch.
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index e2ef1c17942f..65c0ae13215a 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -4554,8 +4554,14 @@ void page_frag_free(void *addr)
> > {
> > struct page *page = virt_to_head_page(addr);
> >
> > - if (unlikely(put_page_testzero(page)))
> > - __free_pages_ok(page, compound_order(page));
> > + if (unlikely(put_page_testzero(page))) {
> > + unsigned int order = compound_order(page);
> > +
> > + if (order == 0)
> > + free_unref_page(page);
> > + else
> > + __free_pages_ok(page, order);
> > + }
> > }
> > EXPORT_SYMBOL(page_frag_free);
>
> Thank you Aaron for spotting this!!!
Which is impossible without your analysis :-)
^ permalink raw reply
* [PATCH] sock_diag: fix autoloading of the raw_diag module
From: Andrei Vagin @ 2018-11-05 6:37 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Andrei Vagin, Cyrill Gorcunov, Xin Long
IPPROTO_TCP isn't registred as an inet protocol, so
inet_protos[protocol] is always NULL for it.
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Xin Long <lucien.xin@gmail.com>
Fixes: bf2ae2e4bf93 ("sock_diag: request _diag module only when the family or proto has been registered")
Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
net/core/sock.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/core/sock.c b/net/core/sock.c
index 6fcc4bc07d19..080a880a1761 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3279,6 +3279,7 @@ int sock_load_diag_module(int family, int protocol)
#ifdef CONFIG_INET
if (family == AF_INET &&
+ protocol != IPPROTO_RAW &&
!rcu_access_pointer(inet_protos[protocol]))
return -ENOENT;
#endif
--
2.17.2
^ permalink raw reply related
* Re: [PATCH] net: skbuff.h: remove unnecessary unlikely()
From: Frank Lee @ 2018-11-05 16:02 UTC (permalink / raw)
To: davem, Eric Dumazet, Willem de Bruijn, dja, ast, sbrivio, pabeni
Cc: linux-kernel, netdev, Yangtao Li
In-Reply-To: <CAEExFWuGN_R=7B4ueAVA8hVoix8ko8zQSXzHxZB5gjwP7jOjpg@mail.gmail.com>
add myself.
-Yangtao
On Mon, Nov 5, 2018 at 9:21 PM Frank Lee <tiny.windzz@gmail.com> wrote:
>
> add netdev@vger.kernel.org
> -- Yangtao
> On Sun, Nov 4, 2018 at 9:14 PM Yangtao Li <tiny.windzz@gmail.com> wrote:
> >
> > WARN_ON() already contains an unlikely(), so it's not necessary to use
> > unlikely.
> >
> > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> > ---
> > include/linux/skbuff.h | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 0ba687454267..7dcfb5591dc3 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -2508,10 +2508,8 @@ int ___pskb_trim(struct sk_buff *skb, unsigned int len);
> >
> > static inline void __skb_set_length(struct sk_buff *skb, unsigned int len)
> > {
> > - if (unlikely(skb_is_nonlinear(skb))) {
> > - WARN_ON(1);
> > + if (WARN_ON(skb_is_nonlinear(skb)))
> > return;
> > - }
> > skb->len = len;
> > skb_set_tail_pointer(skb, len);
> > }
> > --
> > 2.17.0
> >
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox