As the 32-bit arch with 64-bit DMA seems to rare those days,
and page pool might carry a lot of code and complexity for
systems that possibly.
So disable dma mapping support for such systems, if drivers
really want to work on such systems, they have to implement
their own DMA-mapping fallback tracking outside page_pool.
Reviewed-by: Ilias Apalodimas <[email protected]>
Signed-off-by: Yunsheng Lin <[email protected]>
---
V6: Drop pp page tracking support
---
include/linux/mm_types.h | 13 +------------
include/net/page_pool.h | 12 +-----------
net/core/page_pool.c | 10 ++++++----
3 files changed, 8 insertions(+), 27 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 7f8ee09c711f..436e0946d691 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -104,18 +104,7 @@ struct page {
struct page_pool *pp;
unsigned long _pp_mapping_pad;
unsigned long dma_addr;
- union {
- /**
- * dma_addr_upper: might require a 64-bit
- * value on 32-bit architectures.
- */
- unsigned long dma_addr_upper;
- /**
- * For frag page support, not supported in
- * 32-bit architectures with 64-bit DMA.
- */
- atomic_long_t pp_frag_count;
- };
+ atomic_long_t pp_frag_count;
};
struct { /* slab, slob and slub */
union {
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index a4082406a003..3855f069627f 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
page_pool_put_full_page(pool, page, true);
}
-#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
- (sizeof(dma_addr_t) > sizeof(unsigned long))
-
static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
{
- dma_addr_t ret = page->dma_addr;
-
- if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
- ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
-
- return ret;
+ return page->dma_addr;
}
static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
{
page->dma_addr = addr;
- if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
- page->dma_addr_upper = upper_32_bits(addr);
}
static inline void page_pool_set_frag_count(struct page *page, long nr)
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 1a6978427d6c..9b60e4301a44 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
* which is the XDP_TX use-case.
*/
if (pool->p.flags & PP_FLAG_DMA_MAP) {
+ /* DMA-mapping is not supported on 32-bit systems with
+ * 64-bit DMA mapping.
+ */
+ if (sizeof(dma_addr_t) > sizeof(unsigned long))
+ return -EOPNOTSUPP;
+
if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
(pool->p.dma_dir != DMA_BIDIRECTIONAL))
return -EINVAL;
@@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
*/
}
- if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
- pool->p.flags & PP_FLAG_PAGE_FRAG)
- return -EINVAL;
-
if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
return -ENOMEM;
--
2.33.0
On 13/10/2021 11.19, Yunsheng Lin wrote:
> As the 32-bit arch with 64-bit DMA seems to rare those days,
> and page pool might carry a lot of code and complexity for
> systems that possibly.
>
> So disable dma mapping support for such systems, if drivers
> really want to work on such systems, they have to implement
> their own DMA-mapping fallback tracking outside page_pool.
>
> Reviewed-by: Ilias Apalodimas <[email protected]>
> Signed-off-by: Yunsheng Lin <[email protected]>
> ---
> V6: Drop pp page tracking support
> ---
> include/linux/mm_types.h | 13 +------------
> include/net/page_pool.h | 12 +-----------
> net/core/page_pool.c | 10 ++++++----
> 3 files changed, 8 insertions(+), 27 deletions(-)
Acked-by: Jesper Dangaard Brouer <[email protected]>
This is a nice simplification of struct page and page_pool code, when we
don't need to handle this 32-bit ARCH with 64-bit DMA case.
It also gets rid of the confusingly named define. Thanks.
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 7f8ee09c711f..436e0946d691 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -104,18 +104,7 @@ struct page {
> struct page_pool *pp;
> unsigned long _pp_mapping_pad;
> unsigned long dma_addr;
> - union {
> - /**
> - * dma_addr_upper: might require a 64-bit
> - * value on 32-bit architectures.
> - */
> - unsigned long dma_addr_upper;
> - /**
> - * For frag page support, not supported in
> - * 32-bit architectures with 64-bit DMA.
> - */
> - atomic_long_t pp_frag_count;
> - };
> + atomic_long_t pp_frag_count;
> };
> struct { /* slab, slob and slub */
> union {
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index a4082406a003..3855f069627f 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
> page_pool_put_full_page(pool, page, true);
> }
>
> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
> - (sizeof(dma_addr_t) > sizeof(unsigned long))
> -
> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> {
> - dma_addr_t ret = page->dma_addr;
> -
> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
> -
> - return ret;
> + return page->dma_addr;
> }
>
> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
> {
> page->dma_addr = addr;
> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> - page->dma_addr_upper = upper_32_bits(addr);
> }
>
> static inline void page_pool_set_frag_count(struct page *page, long nr)
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 1a6978427d6c..9b60e4301a44 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
> * which is the XDP_TX use-case.
> */
> if (pool->p.flags & PP_FLAG_DMA_MAP) {
> + /* DMA-mapping is not supported on 32-bit systems with
> + * 64-bit DMA mapping.
> + */
> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> + return -EOPNOTSUPP;
> +
> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
> return -EINVAL;
> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
> */
> }
>
> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
> - pool->p.flags & PP_FLAG_PAGE_FRAG)
> - return -EINVAL;
> -
> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
> return -ENOMEM;
>
>
Hello:
This patch was applied to netdev/net-next.git (master)
by David S. Miller <[email protected]>:
On Wed, 13 Oct 2021 17:19:20 +0800 you wrote:
> As the 32-bit arch with 64-bit DMA seems to rare those days,
> and page pool might carry a lot of code and complexity for
> systems that possibly.
>
> So disable dma mapping support for such systems, if drivers
> really want to work on such systems, they have to implement
> their own DMA-mapping fallback tracking outside page_pool.
>
> [...]
Here is the summary with links:
- [net-next,v6] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
https://git.kernel.org/netdev/net-next/c/d00e60ee54b1
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Hi Yunsheng,
Please see the bisection report below about a boot failure on
rk3288-rock2-square which is pointing to this patch. The issue
appears to only happen with CONFIG_ARM_LPAE=y.
Reports aren't automatically sent to the public while we're
trialing new bisection features on kernelci.org but this one
looks valid.
Some more details can be found here:
https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
Here's the same revision on the same platform booting fine with a
plain multi_v7_defconfig build:
https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
Please let us know if you need any help debugging this issue or
if you have a fix to try.
Best wishes,
Guillaume
GitHub: https://github.com/kernelci/kernelci-project/issues/71
-------------------------------------------------------------------------------
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* This automated bisection report was sent to you on the basis *
* that you may be involved with the breaking commit it has *
* found. No manual investigation has been done to verify it, *
* and the root cause of the problem may be somewhere else. *
* *
* If you do send a fix, please include this trailer: *
* Reported-by: "kernelci.org bot" <[email protected]> *
* *
* Hope this helps! *
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
mainline/master bisection: baseline.login on rk3288-rock2-square
Summary:
Start: e851dfae4371d Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
Plain log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.txt
HTML log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.html
Result: d00e60ee54b12 page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
Checks:
revert: PASS
verify: PASS
Parameters:
Tree: mainline
URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Branch: master
Target: rk3288-rock2-square
CPU arch: arm
Lab: lab-collabora
Compiler: gcc-10
Config: multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y
Test case: baseline.login
Breaking commit found:
-------------------------------------------------------------------------------
commit d00e60ee54b12de945b8493cf18c1ada9e422514
Author: Yunsheng Lin <[email protected]>
Date: Wed Oct 13 17:19:20 2021 +0800
page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
On 13/10/2021 10:19, Yunsheng Lin wrote:
> As the 32-bit arch with 64-bit DMA seems to rare those days,
> and page pool might carry a lot of code and complexity for
> systems that possibly.
>
> So disable dma mapping support for such systems, if drivers
> really want to work on such systems, they have to implement
> their own DMA-mapping fallback tracking outside page_pool.
>
> Reviewed-by: Ilias Apalodimas <[email protected]>
> Signed-off-by: Yunsheng Lin <[email protected]>
> ---
> V6: Drop pp page tracking support
> ---
> include/linux/mm_types.h | 13 +------------
> include/net/page_pool.h | 12 +-----------
> net/core/page_pool.c | 10 ++++++----
> 3 files changed, 8 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 7f8ee09c711f..436e0946d691 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -104,18 +104,7 @@ struct page {
> struct page_pool *pp;
> unsigned long _pp_mapping_pad;
> unsigned long dma_addr;
> - union {
> - /**
> - * dma_addr_upper: might require a 64-bit
> - * value on 32-bit architectures.
> - */
> - unsigned long dma_addr_upper;
> - /**
> - * For frag page support, not supported in
> - * 32-bit architectures with 64-bit DMA.
> - */
> - atomic_long_t pp_frag_count;
> - };
> + atomic_long_t pp_frag_count;
> };
> struct { /* slab, slob and slub */
> union {
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index a4082406a003..3855f069627f 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
> page_pool_put_full_page(pool, page, true);
> }
>
> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
> - (sizeof(dma_addr_t) > sizeof(unsigned long))
> -
> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> {
> - dma_addr_t ret = page->dma_addr;
> -
> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
> -
> - return ret;
> + return page->dma_addr;
> }
>
> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
> {
> page->dma_addr = addr;
> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> - page->dma_addr_upper = upper_32_bits(addr);
> }
>
> static inline void page_pool_set_frag_count(struct page *page, long nr)
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 1a6978427d6c..9b60e4301a44 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
> * which is the XDP_TX use-case.
> */
> if (pool->p.flags & PP_FLAG_DMA_MAP) {
> + /* DMA-mapping is not supported on 32-bit systems with
> + * 64-bit DMA mapping.
> + */
> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> + return -EOPNOTSUPP;
> +
> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
> return -EINVAL;
> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
> */
> }
>
> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
> - pool->p.flags & PP_FLAG_PAGE_FRAG)
> - return -EINVAL;
> -
> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
> return -ENOMEM;
>
>
Git bisection log:
-------------------------------------------------------------------------------
git bisect start
# good: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
git bisect good bfc484fe6abba4b89ec9330e0e68778e2a9856b2
# bad: [e851dfae4371d3c751f1e18e8eb5eba993de1467] Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
git bisect bad e851dfae4371d3c751f1e18e8eb5eba993de1467
# bad: [dcd68326d29b62f3039e4f4d23d3e38f24d37360] Merge tag 'devicetree-for-5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
git bisect bad dcd68326d29b62f3039e4f4d23d3e38f24d37360
# bad: [b7b98f868987cd3e86c9bd9a6db048614933d7a0] Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
git bisect bad b7b98f868987cd3e86c9bd9a6db048614933d7a0
# bad: [9fd3d5dced976640f588e0a866b9611db2d2cb37] net: ethernet: ave: Add compatible string and SoC-dependent data for NX1 SoC
git bisect bad 9fd3d5dced976640f588e0a866b9611db2d2cb37
# good: [a96d317fb1a30b9f323548eb2ff05d4e4600ead9] ethernet: use eth_hw_addr_set()
git bisect good a96d317fb1a30b9f323548eb2ff05d4e4600ead9
# good: [f5396b8a663f7a78ee5b75a47ee524b40795b265] ice: switchdev slow path
git bisect good f5396b8a663f7a78ee5b75a47ee524b40795b265
# good: [20c3d9e45ba630a7156d682a40988c0e96be1b92] hamradio: use dev_addr_set() for setting device address
git bisect good 20c3d9e45ba630a7156d682a40988c0e96be1b92
# bad: [a64b442137669c9e839c6a70965989b01b1253b7] net: dpaa2: add support for manual setup of IRQ coalesing
git bisect bad a64b442137669c9e839c6a70965989b01b1253b7
# good: [30fc7efa38f21afa48b0be6bf2053e4c10ae2c78] net, neigh: Reject creating NUD_PERMANENT with NTF_MANAGED entries
git bisect good 30fc7efa38f21afa48b0be6bf2053e4c10ae2c78
# bad: [13ad5ccc093ff448b99ac7e138e91e78796adb48] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
git bisect bad 13ad5ccc093ff448b99ac7e138e91e78796adb48
# good: [40088915f547b52635f022c1e1e18df65ae3153a] Merge branch 'octeontx2-af-miscellaneous-changes-for-cpt'
git bisect good 40088915f547b52635f022c1e1e18df65ae3153a
# bad: [fdbf35df9c091db9c46e57e9938e3f7a4f603a7c] dt-bindings: net: dsa: qca8k: Add SGMII clock phase properties
git bisect bad fdbf35df9c091db9c46e57e9938e3f7a4f603a7c
# bad: [bacc8daf97d4199316328a5d18eeafbe447143c5] xen-netback: Remove redundant initialization of variable err
git bisect bad bacc8daf97d4199316328a5d18eeafbe447143c5
# bad: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
git bisect bad d00e60ee54b12de945b8493cf18c1ada9e422514
# first bad commit: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
-------------------------------------------------------------------------------
On 2021/11/9 17:58, Guillaume Tucker wrote:
> Hi Yunsheng,
>
> Please see the bisection report below about a boot failure on
> rk3288-rock2-square which is pointing to this patch. The issue
> appears to only happen with CONFIG_ARM_LPAE=y.
>
> Reports aren't automatically sent to the public while we're
> trialing new bisection features on kernelci.org but this one
> looks valid.
>
> Some more details can be found here:
>
> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
>
> Here's the same revision on the same platform booting fine with a
> plain multi_v7_defconfig build:
>
> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
>
> Please let us know if you need any help debugging this issue or
> if you have a fix to try.
The patch below is removing the dma mapping support in page pool
for 32 bit systems with 64 bit dma address, so it seems there
is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
flags set in a 32 bit systems with 64 bit dma address.
It seems we might need to revert the below patch or implement the
DMA-mapping tracking support in the driver as mentioned in the below
commit log.
which ethernet driver do you use in your system?
>
> Best wishes,
> Guillaume
>
>
> GitHub: https://github.com/kernelci/kernelci-project/issues/71
>
> -------------------------------------------------------------------------------
>
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> * This automated bisection report was sent to you on the basis *
> * that you may be involved with the breaking commit it has *
> * found. No manual investigation has been done to verify it, *
> * and the root cause of the problem may be somewhere else. *
> * *
> * If you do send a fix, please include this trailer: *
> * Reported-by: "kernelci.org bot" <[email protected]> *
> * *
> * Hope this helps! *
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>
> mainline/master bisection: baseline.login on rk3288-rock2-square
>
> Summary:
> Start: e851dfae4371d Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
> Plain log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.txt
> HTML log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.html
> Result: d00e60ee54b12 page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>
> Checks:
> revert: PASS
> verify: PASS
>
> Parameters:
> Tree: mainline
> URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> Branch: master
> Target: rk3288-rock2-square
> CPU arch: arm
> Lab: lab-collabora
> Compiler: gcc-10
> Config: multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y
> Test case: baseline.login
>
> Breaking commit found:
>
> -------------------------------------------------------------------------------
> commit d00e60ee54b12de945b8493cf18c1ada9e422514
> Author: Yunsheng Lin <[email protected]>
> Date: Wed Oct 13 17:19:20 2021 +0800
>
> page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>
>
> On 13/10/2021 10:19, Yunsheng Lin wrote:
>> As the 32-bit arch with 64-bit DMA seems to rare those days,
>> and page pool might carry a lot of code and complexity for
>> systems that possibly.
>>
>> So disable dma mapping support for such systems, if drivers
>> really want to work on such systems, they have to implement
>> their own DMA-mapping fallback tracking outside page_pool.
>>
>> Reviewed-by: Ilias Apalodimas <[email protected]>
>> Signed-off-by: Yunsheng Lin <[email protected]>
>> ---
>> V6: Drop pp page tracking support
>> ---
>> include/linux/mm_types.h | 13 +------------
>> include/net/page_pool.h | 12 +-----------
>> net/core/page_pool.c | 10 ++++++----
>> 3 files changed, 8 insertions(+), 27 deletions(-)
>>
>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>> index 7f8ee09c711f..436e0946d691 100644
>> --- a/include/linux/mm_types.h
>> +++ b/include/linux/mm_types.h
>> @@ -104,18 +104,7 @@ struct page {
>> struct page_pool *pp;
>> unsigned long _pp_mapping_pad;
>> unsigned long dma_addr;
>> - union {
>> - /**
>> - * dma_addr_upper: might require a 64-bit
>> - * value on 32-bit architectures.
>> - */
>> - unsigned long dma_addr_upper;
>> - /**
>> - * For frag page support, not supported in
>> - * 32-bit architectures with 64-bit DMA.
>> - */
>> - atomic_long_t pp_frag_count;
>> - };
>> + atomic_long_t pp_frag_count;
>> };
>> struct { /* slab, slob and slub */
>> union {
>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>> index a4082406a003..3855f069627f 100644
>> --- a/include/net/page_pool.h
>> +++ b/include/net/page_pool.h
>> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>> page_pool_put_full_page(pool, page, true);
>> }
>>
>> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
>> - (sizeof(dma_addr_t) > sizeof(unsigned long))
>> -
>> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>> {
>> - dma_addr_t ret = page->dma_addr;
>> -
>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
>> -
>> - return ret;
>> + return page->dma_addr;
>> }
>>
>> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
>> {
>> page->dma_addr = addr;
>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>> - page->dma_addr_upper = upper_32_bits(addr);
>> }
>>
>> static inline void page_pool_set_frag_count(struct page *page, long nr)
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index 1a6978427d6c..9b60e4301a44 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
>> * which is the XDP_TX use-case.
>> */
>> if (pool->p.flags & PP_FLAG_DMA_MAP) {
>> + /* DMA-mapping is not supported on 32-bit systems with
>> + * 64-bit DMA mapping.
>> + */
>> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
>> + return -EOPNOTSUPP;
>> +
>> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
>> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
>> return -EINVAL;
>> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
>> */
>> }
>>
>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
>> - pool->p.flags & PP_FLAG_PAGE_FRAG)
>> - return -EINVAL;
>> -
>> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
>> return -ENOMEM;
>>
>>
>
>
>
> Git bisection log:
>
> -------------------------------------------------------------------------------
> git bisect start
> # good: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
> git bisect good bfc484fe6abba4b89ec9330e0e68778e2a9856b2
> # bad: [e851dfae4371d3c751f1e18e8eb5eba993de1467] Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
> git bisect bad e851dfae4371d3c751f1e18e8eb5eba993de1467
> # bad: [dcd68326d29b62f3039e4f4d23d3e38f24d37360] Merge tag 'devicetree-for-5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
> git bisect bad dcd68326d29b62f3039e4f4d23d3e38f24d37360
> # bad: [b7b98f868987cd3e86c9bd9a6db048614933d7a0] Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
> git bisect bad b7b98f868987cd3e86c9bd9a6db048614933d7a0
> # bad: [9fd3d5dced976640f588e0a866b9611db2d2cb37] net: ethernet: ave: Add compatible string and SoC-dependent data for NX1 SoC
> git bisect bad 9fd3d5dced976640f588e0a866b9611db2d2cb37
> # good: [a96d317fb1a30b9f323548eb2ff05d4e4600ead9] ethernet: use eth_hw_addr_set()
> git bisect good a96d317fb1a30b9f323548eb2ff05d4e4600ead9
> # good: [f5396b8a663f7a78ee5b75a47ee524b40795b265] ice: switchdev slow path
> git bisect good f5396b8a663f7a78ee5b75a47ee524b40795b265
> # good: [20c3d9e45ba630a7156d682a40988c0e96be1b92] hamradio: use dev_addr_set() for setting device address
> git bisect good 20c3d9e45ba630a7156d682a40988c0e96be1b92
> # bad: [a64b442137669c9e839c6a70965989b01b1253b7] net: dpaa2: add support for manual setup of IRQ coalesing
> git bisect bad a64b442137669c9e839c6a70965989b01b1253b7
> # good: [30fc7efa38f21afa48b0be6bf2053e4c10ae2c78] net, neigh: Reject creating NUD_PERMANENT with NTF_MANAGED entries
> git bisect good 30fc7efa38f21afa48b0be6bf2053e4c10ae2c78
> # bad: [13ad5ccc093ff448b99ac7e138e91e78796adb48] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
> git bisect bad 13ad5ccc093ff448b99ac7e138e91e78796adb48
> # good: [40088915f547b52635f022c1e1e18df65ae3153a] Merge branch 'octeontx2-af-miscellaneous-changes-for-cpt'
> git bisect good 40088915f547b52635f022c1e1e18df65ae3153a
> # bad: [fdbf35df9c091db9c46e57e9938e3f7a4f603a7c] dt-bindings: net: dsa: qca8k: Add SGMII clock phase properties
> git bisect bad fdbf35df9c091db9c46e57e9938e3f7a4f603a7c
> # bad: [bacc8daf97d4199316328a5d18eeafbe447143c5] xen-netback: Remove redundant initialization of variable err
> git bisect bad bacc8daf97d4199316328a5d18eeafbe447143c5
> # bad: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> git bisect bad d00e60ee54b12de945b8493cf18c1ada9e422514
> # first bad commit: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> -------------------------------------------------------------------------------
> .
>
On 09/11/2021 12:02, Yunsheng Lin wrote:
> On 2021/11/9 17:58, Guillaume Tucker wrote:
>> Hi Yunsheng,
>>
>> Please see the bisection report below about a boot failure on
>> rk3288-rock2-square which is pointing to this patch. The issue
>> appears to only happen with CONFIG_ARM_LPAE=y.
>>
>> Reports aren't automatically sent to the public while we're
>> trialing new bisection features on kernelci.org but this one
>> looks valid.
>>
>> Some more details can be found here:
>>
>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
>>
>> Here's the same revision on the same platform booting fine with a
>> plain multi_v7_defconfig build:
>>
>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
>>
>> Please let us know if you need any help debugging this issue or
>> if you have a fix to try.
>
> The patch below is removing the dma mapping support in page pool
> for 32 bit systems with 64 bit dma address, so it seems there
> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
> flags set in a 32 bit systems with 64 bit dma address.
>
> It seems we might need to revert the below patch or implement the
> DMA-mapping tracking support in the driver as mentioned in the below
> commit log.
>
> which ethernet driver do you use in your system?
Thanks for taking a look and sorry for the slow reply. Here's a
booting test job with LPAE disabled:
https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
[ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
Best wishes,
Guillaume
>> GitHub: https://github.com/kernelci/kernelci-project/issues/71
>>
>> -------------------------------------------------------------------------------
>>
>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>> * This automated bisection report was sent to you on the basis *
>> * that you may be involved with the breaking commit it has *
>> * found. No manual investigation has been done to verify it, *
>> * and the root cause of the problem may be somewhere else. *
>> * *
>> * If you do send a fix, please include this trailer: *
>> * Reported-by: "kernelci.org bot" <[email protected]> *
>> * *
>> * Hope this helps! *
>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>>
>> mainline/master bisection: baseline.login on rk3288-rock2-square
>>
>> Summary:
>> Start: e851dfae4371d Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
>> Plain log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.txt
>> HTML log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.html
>> Result: d00e60ee54b12 page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>
>> Checks:
>> revert: PASS
>> verify: PASS
>>
>> Parameters:
>> Tree: mainline
>> URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> Branch: master
>> Target: rk3288-rock2-square
>> CPU arch: arm
>> Lab: lab-collabora
>> Compiler: gcc-10
>> Config: multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y
>> Test case: baseline.login
>>
>> Breaking commit found:
>>
>> -------------------------------------------------------------------------------
>> commit d00e60ee54b12de945b8493cf18c1ada9e422514
>> Author: Yunsheng Lin <[email protected]>
>> Date: Wed Oct 13 17:19:20 2021 +0800
>>
>> page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>
>>
>> On 13/10/2021 10:19, Yunsheng Lin wrote:
>>> As the 32-bit arch with 64-bit DMA seems to rare those days,
>>> and page pool might carry a lot of code and complexity for
>>> systems that possibly.
>>>
>>> So disable dma mapping support for such systems, if drivers
>>> really want to work on such systems, they have to implement
>>> their own DMA-mapping fallback tracking outside page_pool.
>>>
>>> Reviewed-by: Ilias Apalodimas <[email protected]>
>>> Signed-off-by: Yunsheng Lin <[email protected]>
>>> ---
>>> V6: Drop pp page tracking support
>>> ---
>>> include/linux/mm_types.h | 13 +------------
>>> include/net/page_pool.h | 12 +-----------
>>> net/core/page_pool.c | 10 ++++++----
>>> 3 files changed, 8 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>>> index 7f8ee09c711f..436e0946d691 100644
>>> --- a/include/linux/mm_types.h
>>> +++ b/include/linux/mm_types.h
>>> @@ -104,18 +104,7 @@ struct page {
>>> struct page_pool *pp;
>>> unsigned long _pp_mapping_pad;
>>> unsigned long dma_addr;
>>> - union {
>>> - /**
>>> - * dma_addr_upper: might require a 64-bit
>>> - * value on 32-bit architectures.
>>> - */
>>> - unsigned long dma_addr_upper;
>>> - /**
>>> - * For frag page support, not supported in
>>> - * 32-bit architectures with 64-bit DMA.
>>> - */
>>> - atomic_long_t pp_frag_count;
>>> - };
>>> + atomic_long_t pp_frag_count;
>>> };
>>> struct { /* slab, slob and slub */
>>> union {
>>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>>> index a4082406a003..3855f069627f 100644
>>> --- a/include/net/page_pool.h
>>> +++ b/include/net/page_pool.h
>>> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>>> page_pool_put_full_page(pool, page, true);
>>> }
>>>
>>> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
>>> - (sizeof(dma_addr_t) > sizeof(unsigned long))
>>> -
>>> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>>> {
>>> - dma_addr_t ret = page->dma_addr;
>>> -
>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>>> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
>>> -
>>> - return ret;
>>> + return page->dma_addr;
>>> }
>>>
>>> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
>>> {
>>> page->dma_addr = addr;
>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>>> - page->dma_addr_upper = upper_32_bits(addr);
>>> }
>>>
>>> static inline void page_pool_set_frag_count(struct page *page, long nr)
>>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>>> index 1a6978427d6c..9b60e4301a44 100644
>>> --- a/net/core/page_pool.c
>>> +++ b/net/core/page_pool.c
>>> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
>>> * which is the XDP_TX use-case.
>>> */
>>> if (pool->p.flags & PP_FLAG_DMA_MAP) {
>>> + /* DMA-mapping is not supported on 32-bit systems with
>>> + * 64-bit DMA mapping.
>>> + */
>>> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
>>> + return -EOPNOTSUPP;
>>> +
>>> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
>>> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
>>> return -EINVAL;
>>> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
>>> */
>>> }
>>>
>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
>>> - pool->p.flags & PP_FLAG_PAGE_FRAG)
>>> - return -EINVAL;
>>> -
>>> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
>>> return -ENOMEM;
>>>
>>>
>>
>>
>>
>> Git bisection log:
>>
>> -------------------------------------------------------------------------------
>> git bisect start
>> # good: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
>> git bisect good bfc484fe6abba4b89ec9330e0e68778e2a9856b2
>> # bad: [e851dfae4371d3c751f1e18e8eb5eba993de1467] Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
>> git bisect bad e851dfae4371d3c751f1e18e8eb5eba993de1467
>> # bad: [dcd68326d29b62f3039e4f4d23d3e38f24d37360] Merge tag 'devicetree-for-5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
>> git bisect bad dcd68326d29b62f3039e4f4d23d3e38f24d37360
>> # bad: [b7b98f868987cd3e86c9bd9a6db048614933d7a0] Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
>> git bisect bad b7b98f868987cd3e86c9bd9a6db048614933d7a0
>> # bad: [9fd3d5dced976640f588e0a866b9611db2d2cb37] net: ethernet: ave: Add compatible string and SoC-dependent data for NX1 SoC
>> git bisect bad 9fd3d5dced976640f588e0a866b9611db2d2cb37
>> # good: [a96d317fb1a30b9f323548eb2ff05d4e4600ead9] ethernet: use eth_hw_addr_set()
>> git bisect good a96d317fb1a30b9f323548eb2ff05d4e4600ead9
>> # good: [f5396b8a663f7a78ee5b75a47ee524b40795b265] ice: switchdev slow path
>> git bisect good f5396b8a663f7a78ee5b75a47ee524b40795b265
>> # good: [20c3d9e45ba630a7156d682a40988c0e96be1b92] hamradio: use dev_addr_set() for setting device address
>> git bisect good 20c3d9e45ba630a7156d682a40988c0e96be1b92
>> # bad: [a64b442137669c9e839c6a70965989b01b1253b7] net: dpaa2: add support for manual setup of IRQ coalesing
>> git bisect bad a64b442137669c9e839c6a70965989b01b1253b7
>> # good: [30fc7efa38f21afa48b0be6bf2053e4c10ae2c78] net, neigh: Reject creating NUD_PERMANENT with NTF_MANAGED entries
>> git bisect good 30fc7efa38f21afa48b0be6bf2053e4c10ae2c78
>> # bad: [13ad5ccc093ff448b99ac7e138e91e78796adb48] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
>> git bisect bad 13ad5ccc093ff448b99ac7e138e91e78796adb48
>> # good: [40088915f547b52635f022c1e1e18df65ae3153a] Merge branch 'octeontx2-af-miscellaneous-changes-for-cpt'
>> git bisect good 40088915f547b52635f022c1e1e18df65ae3153a
>> # bad: [fdbf35df9c091db9c46e57e9938e3f7a4f603a7c] dt-bindings: net: dsa: qca8k: Add SGMII clock phase properties
>> git bisect bad fdbf35df9c091db9c46e57e9938e3f7a4f603a7c
>> # bad: [bacc8daf97d4199316328a5d18eeafbe447143c5] xen-netback: Remove redundant initialization of variable err
>> git bisect bad bacc8daf97d4199316328a5d18eeafbe447143c5
>> # bad: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>> git bisect bad d00e60ee54b12de945b8493cf18c1ada9e422514
>> # first bad commit: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>> -------------------------------------------------------------------------------
>> .
>>
On 2021/11/12 17:21, Guillaume Tucker wrote:
> On 09/11/2021 12:02, Yunsheng Lin wrote:
>> On 2021/11/9 17:58, Guillaume Tucker wrote:
>>> Hi Yunsheng,
>>>
>>> Please see the bisection report below about a boot failure on
>>> rk3288-rock2-square which is pointing to this patch. The issue
>>> appears to only happen with CONFIG_ARM_LPAE=y.
>>>
>>> Reports aren't automatically sent to the public while we're
>>> trialing new bisection features on kernelci.org but this one
>>> looks valid.
>>>
>>> Some more details can be found here:
>>>
>>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
>>>
>>> Here's the same revision on the same platform booting fine with a
>>> plain multi_v7_defconfig build:
>>>
>>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
>>>
>>> Please let us know if you need any help debugging this issue or
>>> if you have a fix to try.
>>
>> The patch below is removing the dma mapping support in page pool
>> for 32 bit systems with 64 bit dma address, so it seems there
>> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
>> flags set in a 32 bit systems with 64 bit dma address.
>>
>> It seems we might need to revert the below patch or implement the
>> DMA-mapping tracking support in the driver as mentioned in the below
>> commit log.
>>
>> which ethernet driver do you use in your system?
>
> Thanks for taking a look and sorry for the slow reply. Here's a
> booting test job with LPAE disabled:
>
> https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
> https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
>
> [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>
> So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
Thanks for the report, this patch seems to cause problem for 32-bit
system with LPAE enabled.
As LPAE seems like a common feature for 32 bits system, this patch
might need to be reverted.
@Jesper, @Ilias, what do you think?
>
> Best wishes,
> Guillaume
>
>
>>> GitHub: https://github.com/kernelci/kernelci-project/issues/71
>>>
>>> -------------------------------------------------------------------------------
>>>
>>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>>> * This automated bisection report was sent to you on the basis *
>>> * that you may be involved with the breaking commit it has *
>>> * found. No manual investigation has been done to verify it, *
>>> * and the root cause of the problem may be somewhere else. *
>>> * *
>>> * If you do send a fix, please include this trailer: *
>>> * Reported-by: "kernelci.org bot" <[email protected]> *
>>> * *
>>> * Hope this helps! *
>>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>>>
>>> mainline/master bisection: baseline.login on rk3288-rock2-square
>>>
>>> Summary:
>>> Start: e851dfae4371d Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
>>> Plain log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.txt
>>> HTML log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.html
>>> Result: d00e60ee54b12 page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>>
>>> Checks:
>>> revert: PASS
>>> verify: PASS
>>>
>>> Parameters:
>>> Tree: mainline
>>> URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>> Branch: master
>>> Target: rk3288-rock2-square
>>> CPU arch: arm
>>> Lab: lab-collabora
>>> Compiler: gcc-10
>>> Config: multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y
>>> Test case: baseline.login
>>>
>>> Breaking commit found:
>>>
>>> -------------------------------------------------------------------------------
>>> commit d00e60ee54b12de945b8493cf18c1ada9e422514
>>> Author: Yunsheng Lin <[email protected]>
>>> Date: Wed Oct 13 17:19:20 2021 +0800
>>>
>>> page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>>
>>>
>>> On 13/10/2021 10:19, Yunsheng Lin wrote:
>>>> As the 32-bit arch with 64-bit DMA seems to rare those days,
>>>> and page pool might carry a lot of code and complexity for
>>>> systems that possibly.
>>>>
>>>> So disable dma mapping support for such systems, if drivers
>>>> really want to work on such systems, they have to implement
>>>> their own DMA-mapping fallback tracking outside page_pool.
>>>>
>>>> Reviewed-by: Ilias Apalodimas <[email protected]>
>>>> Signed-off-by: Yunsheng Lin <[email protected]>
>>>> ---
>>>> V6: Drop pp page tracking support
>>>> ---
>>>> include/linux/mm_types.h | 13 +------------
>>>> include/net/page_pool.h | 12 +-----------
>>>> net/core/page_pool.c | 10 ++++++----
>>>> 3 files changed, 8 insertions(+), 27 deletions(-)
>>>>
>>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>>>> index 7f8ee09c711f..436e0946d691 100644
>>>> --- a/include/linux/mm_types.h
>>>> +++ b/include/linux/mm_types.h
>>>> @@ -104,18 +104,7 @@ struct page {
>>>> struct page_pool *pp;
>>>> unsigned long _pp_mapping_pad;
>>>> unsigned long dma_addr;
>>>> - union {
>>>> - /**
>>>> - * dma_addr_upper: might require a 64-bit
>>>> - * value on 32-bit architectures.
>>>> - */
>>>> - unsigned long dma_addr_upper;
>>>> - /**
>>>> - * For frag page support, not supported in
>>>> - * 32-bit architectures with 64-bit DMA.
>>>> - */
>>>> - atomic_long_t pp_frag_count;
>>>> - };
>>>> + atomic_long_t pp_frag_count;
>>>> };
>>>> struct { /* slab, slob and slub */
>>>> union {
>>>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>>>> index a4082406a003..3855f069627f 100644
>>>> --- a/include/net/page_pool.h
>>>> +++ b/include/net/page_pool.h
>>>> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>>>> page_pool_put_full_page(pool, page, true);
>>>> }
>>>>
>>>> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
>>>> - (sizeof(dma_addr_t) > sizeof(unsigned long))
>>>> -
>>>> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>>>> {
>>>> - dma_addr_t ret = page->dma_addr;
>>>> -
>>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>>>> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
>>>> -
>>>> - return ret;
>>>> + return page->dma_addr;
>>>> }
>>>>
>>>> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
>>>> {
>>>> page->dma_addr = addr;
>>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>>>> - page->dma_addr_upper = upper_32_bits(addr);
>>>> }
>>>>
>>>> static inline void page_pool_set_frag_count(struct page *page, long nr)
>>>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>>>> index 1a6978427d6c..9b60e4301a44 100644
>>>> --- a/net/core/page_pool.c
>>>> +++ b/net/core/page_pool.c
>>>> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
>>>> * which is the XDP_TX use-case.
>>>> */
>>>> if (pool->p.flags & PP_FLAG_DMA_MAP) {
>>>> + /* DMA-mapping is not supported on 32-bit systems with
>>>> + * 64-bit DMA mapping.
>>>> + */
>>>> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
>>>> + return -EOPNOTSUPP;
>>>> +
>>>> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
>>>> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
>>>> return -EINVAL;
>>>> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
>>>> */
>>>> }
>>>>
>>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
>>>> - pool->p.flags & PP_FLAG_PAGE_FRAG)
>>>> - return -EINVAL;
>>>> -
>>>> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
>>>> return -ENOMEM;
>>>>
>>>>
>>>
>>>
>>>
>>> Git bisection log:
>>>
>>> -------------------------------------------------------------------------------
>>> git bisect start
>>> # good: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
>>> git bisect good bfc484fe6abba4b89ec9330e0e68778e2a9856b2
>>> # bad: [e851dfae4371d3c751f1e18e8eb5eba993de1467] Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
>>> git bisect bad e851dfae4371d3c751f1e18e8eb5eba993de1467
>>> # bad: [dcd68326d29b62f3039e4f4d23d3e38f24d37360] Merge tag 'devicetree-for-5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
>>> git bisect bad dcd68326d29b62f3039e4f4d23d3e38f24d37360
>>> # bad: [b7b98f868987cd3e86c9bd9a6db048614933d7a0] Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
>>> git bisect bad b7b98f868987cd3e86c9bd9a6db048614933d7a0
>>> # bad: [9fd3d5dced976640f588e0a866b9611db2d2cb37] net: ethernet: ave: Add compatible string and SoC-dependent data for NX1 SoC
>>> git bisect bad 9fd3d5dced976640f588e0a866b9611db2d2cb37
>>> # good: [a96d317fb1a30b9f323548eb2ff05d4e4600ead9] ethernet: use eth_hw_addr_set()
>>> git bisect good a96d317fb1a30b9f323548eb2ff05d4e4600ead9
>>> # good: [f5396b8a663f7a78ee5b75a47ee524b40795b265] ice: switchdev slow path
>>> git bisect good f5396b8a663f7a78ee5b75a47ee524b40795b265
>>> # good: [20c3d9e45ba630a7156d682a40988c0e96be1b92] hamradio: use dev_addr_set() for setting device address
>>> git bisect good 20c3d9e45ba630a7156d682a40988c0e96be1b92
>>> # bad: [a64b442137669c9e839c6a70965989b01b1253b7] net: dpaa2: add support for manual setup of IRQ coalesing
>>> git bisect bad a64b442137669c9e839c6a70965989b01b1253b7
>>> # good: [30fc7efa38f21afa48b0be6bf2053e4c10ae2c78] net, neigh: Reject creating NUD_PERMANENT with NTF_MANAGED entries
>>> git bisect good 30fc7efa38f21afa48b0be6bf2053e4c10ae2c78
>>> # bad: [13ad5ccc093ff448b99ac7e138e91e78796adb48] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
>>> git bisect bad 13ad5ccc093ff448b99ac7e138e91e78796adb48
>>> # good: [40088915f547b52635f022c1e1e18df65ae3153a] Merge branch 'octeontx2-af-miscellaneous-changes-for-cpt'
>>> git bisect good 40088915f547b52635f022c1e1e18df65ae3153a
>>> # bad: [fdbf35df9c091db9c46e57e9938e3f7a4f603a7c] dt-bindings: net: dsa: qca8k: Add SGMII clock phase properties
>>> git bisect bad fdbf35df9c091db9c46e57e9938e3f7a4f603a7c
>>> # bad: [bacc8daf97d4199316328a5d18eeafbe447143c5] xen-netback: Remove redundant initialization of variable err
>>> git bisect bad bacc8daf97d4199316328a5d18eeafbe447143c5
>>> # bad: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>> git bisect bad d00e60ee54b12de945b8493cf18c1ada9e422514
>>> # first bad commit: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>> -------------------------------------------------------------------------------
>>> .
>>>
>
> .
>
On Mon, Nov 15, 2021 at 11:34:59AM +0800, Yunsheng Lin wrote:
> On 2021/11/12 17:21, Guillaume Tucker wrote:
> > On 09/11/2021 12:02, Yunsheng Lin wrote:
> >> On 2021/11/9 17:58, Guillaume Tucker wrote:
> >>> Hi Yunsheng,
> >>>
> >>> Please see the bisection report below about a boot failure on
> >>> rk3288-rock2-square which is pointing to this patch. The issue
> >>> appears to only happen with CONFIG_ARM_LPAE=y.
> >>>
> >>> Reports aren't automatically sent to the public while we're
> >>> trialing new bisection features on kernelci.org but this one
> >>> looks valid.
> >>>
> >>> Some more details can be found here:
> >>>
> >>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
> >>>
> >>> Here's the same revision on the same platform booting fine with a
> >>> plain multi_v7_defconfig build:
> >>>
> >>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
> >>>
> >>> Please let us know if you need any help debugging this issue or
> >>> if you have a fix to try.
> >>
> >> The patch below is removing the dma mapping support in page pool
> >> for 32 bit systems with 64 bit dma address, so it seems there
> >> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
> >> flags set in a 32 bit systems with 64 bit dma address.
> >>
> >> It seems we might need to revert the below patch or implement the
> >> DMA-mapping tracking support in the driver as mentioned in the below
> >> commit log.
> >>
> >> which ethernet driver do you use in your system?
> >
> > Thanks for taking a look and sorry for the slow reply. Here's a
> > booting test job with LPAE disabled:
> >
> > https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
> > https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
> >
> > [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> >
> > So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>
> Thanks for the report, this patch seems to cause problem for 32-bit
> system with LPAE enabled.
>
> As LPAE seems like a common feature for 32 bits system, this patch
> might need to be reverted.
>
> @Jesper, @Ilias, what do you think?
So enabling LPAE also enables CONFIG_ARCH_DMA_ADDR_T_64BIT on that board?
Doing a quick grep only selects that for XEN. I am ok reverting that, but
I think we need to understand how the dma address ended up being 64bit.
Regards
/Ilias
>
> >
> > Best wishes,
> > Guillaume
> >
> >
> >>> GitHub: https://github.com/kernelci/kernelci-project/issues/71
> >>>
> >>> -------------------------------------------------------------------------------
> >>>
> >>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> >>> * This automated bisection report was sent to you on the basis *
> >>> * that you may be involved with the breaking commit it has *
> >>> * found. No manual investigation has been done to verify it, *
> >>> * and the root cause of the problem may be somewhere else. *
> >>> * *
> >>> * If you do send a fix, please include this trailer: *
> >>> * Reported-by: "kernelci.org bot" <[email protected]> *
> >>> * *
> >>> * Hope this helps! *
> >>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> >>>
> >>> mainline/master bisection: baseline.login on rk3288-rock2-square
> >>>
> >>> Summary:
> >>> Start: e851dfae4371d Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
> >>> Plain log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.txt
> >>> HTML log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.html
> >>> Result: d00e60ee54b12 page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> >>>
> >>> Checks:
> >>> revert: PASS
> >>> verify: PASS
> >>>
> >>> Parameters:
> >>> Tree: mainline
> >>> URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >>> Branch: master
> >>> Target: rk3288-rock2-square
> >>> CPU arch: arm
> >>> Lab: lab-collabora
> >>> Compiler: gcc-10
> >>> Config: multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y
> >>> Test case: baseline.login
> >>>
> >>> Breaking commit found:
> >>>
> >>> -------------------------------------------------------------------------------
> >>> commit d00e60ee54b12de945b8493cf18c1ada9e422514
> >>> Author: Yunsheng Lin <[email protected]>
> >>> Date: Wed Oct 13 17:19:20 2021 +0800
> >>>
> >>> page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> >>>
> >>>
> >>> On 13/10/2021 10:19, Yunsheng Lin wrote:
> >>>> As the 32-bit arch with 64-bit DMA seems to rare those days,
> >>>> and page pool might carry a lot of code and complexity for
> >>>> systems that possibly.
> >>>>
> >>>> So disable dma mapping support for such systems, if drivers
> >>>> really want to work on such systems, they have to implement
> >>>> their own DMA-mapping fallback tracking outside page_pool.
> >>>>
> >>>> Reviewed-by: Ilias Apalodimas <[email protected]>
> >>>> Signed-off-by: Yunsheng Lin <[email protected]>
> >>>> ---
> >>>> V6: Drop pp page tracking support
> >>>> ---
> >>>> include/linux/mm_types.h | 13 +------------
> >>>> include/net/page_pool.h | 12 +-----------
> >>>> net/core/page_pool.c | 10 ++++++----
> >>>> 3 files changed, 8 insertions(+), 27 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> >>>> index 7f8ee09c711f..436e0946d691 100644
> >>>> --- a/include/linux/mm_types.h
> >>>> +++ b/include/linux/mm_types.h
> >>>> @@ -104,18 +104,7 @@ struct page {
> >>>> struct page_pool *pp;
> >>>> unsigned long _pp_mapping_pad;
> >>>> unsigned long dma_addr;
> >>>> - union {
> >>>> - /**
> >>>> - * dma_addr_upper: might require a 64-bit
> >>>> - * value on 32-bit architectures.
> >>>> - */
> >>>> - unsigned long dma_addr_upper;
> >>>> - /**
> >>>> - * For frag page support, not supported in
> >>>> - * 32-bit architectures with 64-bit DMA.
> >>>> - */
> >>>> - atomic_long_t pp_frag_count;
> >>>> - };
> >>>> + atomic_long_t pp_frag_count;
> >>>> };
> >>>> struct { /* slab, slob and slub */
> >>>> union {
> >>>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> >>>> index a4082406a003..3855f069627f 100644
> >>>> --- a/include/net/page_pool.h
> >>>> +++ b/include/net/page_pool.h
> >>>> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
> >>>> page_pool_put_full_page(pool, page, true);
> >>>> }
> >>>>
> >>>> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
> >>>> - (sizeof(dma_addr_t) > sizeof(unsigned long))
> >>>> -
> >>>> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> >>>> {
> >>>> - dma_addr_t ret = page->dma_addr;
> >>>> -
> >>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> >>>> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
> >>>> -
> >>>> - return ret;
> >>>> + return page->dma_addr;
> >>>> }
> >>>>
> >>>> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
> >>>> {
> >>>> page->dma_addr = addr;
> >>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> >>>> - page->dma_addr_upper = upper_32_bits(addr);
> >>>> }
> >>>>
> >>>> static inline void page_pool_set_frag_count(struct page *page, long nr)
> >>>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> >>>> index 1a6978427d6c..9b60e4301a44 100644
> >>>> --- a/net/core/page_pool.c
> >>>> +++ b/net/core/page_pool.c
> >>>> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
> >>>> * which is the XDP_TX use-case.
> >>>> */
> >>>> if (pool->p.flags & PP_FLAG_DMA_MAP) {
> >>>> + /* DMA-mapping is not supported on 32-bit systems with
> >>>> + * 64-bit DMA mapping.
> >>>> + */
> >>>> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> >>>> + return -EOPNOTSUPP;
> >>>> +
> >>>> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
> >>>> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
> >>>> return -EINVAL;
> >>>> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
> >>>> */
> >>>> }
> >>>>
> >>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
> >>>> - pool->p.flags & PP_FLAG_PAGE_FRAG)
> >>>> - return -EINVAL;
> >>>> -
> >>>> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
> >>>> return -ENOMEM;
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>> Git bisection log:
> >>>
> >>> -------------------------------------------------------------------------------
> >>> git bisect start
> >>> # good: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
> >>> git bisect good bfc484fe6abba4b89ec9330e0e68778e2a9856b2
> >>> # bad: [e851dfae4371d3c751f1e18e8eb5eba993de1467] Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
> >>> git bisect bad e851dfae4371d3c751f1e18e8eb5eba993de1467
> >>> # bad: [dcd68326d29b62f3039e4f4d23d3e38f24d37360] Merge tag 'devicetree-for-5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
> >>> git bisect bad dcd68326d29b62f3039e4f4d23d3e38f24d37360
> >>> # bad: [b7b98f868987cd3e86c9bd9a6db048614933d7a0] Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
> >>> git bisect bad b7b98f868987cd3e86c9bd9a6db048614933d7a0
> >>> # bad: [9fd3d5dced976640f588e0a866b9611db2d2cb37] net: ethernet: ave: Add compatible string and SoC-dependent data for NX1 SoC
> >>> git bisect bad 9fd3d5dced976640f588e0a866b9611db2d2cb37
> >>> # good: [a96d317fb1a30b9f323548eb2ff05d4e4600ead9] ethernet: use eth_hw_addr_set()
> >>> git bisect good a96d317fb1a30b9f323548eb2ff05d4e4600ead9
> >>> # good: [f5396b8a663f7a78ee5b75a47ee524b40795b265] ice: switchdev slow path
> >>> git bisect good f5396b8a663f7a78ee5b75a47ee524b40795b265
> >>> # good: [20c3d9e45ba630a7156d682a40988c0e96be1b92] hamradio: use dev_addr_set() for setting device address
> >>> git bisect good 20c3d9e45ba630a7156d682a40988c0e96be1b92
> >>> # bad: [a64b442137669c9e839c6a70965989b01b1253b7] net: dpaa2: add support for manual setup of IRQ coalesing
> >>> git bisect bad a64b442137669c9e839c6a70965989b01b1253b7
> >>> # good: [30fc7efa38f21afa48b0be6bf2053e4c10ae2c78] net, neigh: Reject creating NUD_PERMANENT with NTF_MANAGED entries
> >>> git bisect good 30fc7efa38f21afa48b0be6bf2053e4c10ae2c78
> >>> # bad: [13ad5ccc093ff448b99ac7e138e91e78796adb48] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
> >>> git bisect bad 13ad5ccc093ff448b99ac7e138e91e78796adb48
> >>> # good: [40088915f547b52635f022c1e1e18df65ae3153a] Merge branch 'octeontx2-af-miscellaneous-changes-for-cpt'
> >>> git bisect good 40088915f547b52635f022c1e1e18df65ae3153a
> >>> # bad: [fdbf35df9c091db9c46e57e9938e3f7a4f603a7c] dt-bindings: net: dsa: qca8k: Add SGMII clock phase properties
> >>> git bisect bad fdbf35df9c091db9c46e57e9938e3f7a4f603a7c
> >>> # bad: [bacc8daf97d4199316328a5d18eeafbe447143c5] xen-netback: Remove redundant initialization of variable err
> >>> git bisect bad bacc8daf97d4199316328a5d18eeafbe447143c5
> >>> # bad: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> >>> git bisect bad d00e60ee54b12de945b8493cf18c1ada9e422514
> >>> # first bad commit: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> >>> -------------------------------------------------------------------------------
> >>> .
> >>>
> >
> > .
> >
On Mon, 15 Nov 2021 at 13:53, Ilias Apalodimas
<[email protected]> wrote:
>
> On Mon, Nov 15, 2021 at 11:34:59AM +0800, Yunsheng Lin wrote:
> > On 2021/11/12 17:21, Guillaume Tucker wrote:
> > > On 09/11/2021 12:02, Yunsheng Lin wrote:
> > >> On 2021/11/9 17:58, Guillaume Tucker wrote:
> > >>> Hi Yunsheng,
> > >>>
> > >>> Please see the bisection report below about a boot failure on
> > >>> rk3288-rock2-square which is pointing to this patch. The issue
> > >>> appears to only happen with CONFIG_ARM_LPAE=y.
> > >>>
> > >>> Reports aren't automatically sent to the public while we're
> > >>> trialing new bisection features on kernelci.org but this one
> > >>> looks valid.
> > >>>
> > >>> Some more details can be found here:
> > >>>
> > >>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
> > >>>
> > >>> Here's the same revision on the same platform booting fine with a
> > >>> plain multi_v7_defconfig build:
> > >>>
> > >>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
> > >>>
> > >>> Please let us know if you need any help debugging this issue or
> > >>> if you have a fix to try.
> > >>
> > >> The patch below is removing the dma mapping support in page pool
> > >> for 32 bit systems with 64 bit dma address, so it seems there
> > >> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
> > >> flags set in a 32 bit systems with 64 bit dma address.
> > >>
> > >> It seems we might need to revert the below patch or implement the
> > >> DMA-mapping tracking support in the driver as mentioned in the below
> > >> commit log.
> > >>
> > >> which ethernet driver do you use in your system?
> > >
> > > Thanks for taking a look and sorry for the slow reply. Here's a
> > > booting test job with LPAE disabled:
> > >
> > > https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
> > > https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
> > >
> > > [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> > >
> > > So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> >
> > Thanks for the report, this patch seems to cause problem for 32-bit
> > system with LPAE enabled.
> >
> > As LPAE seems like a common feature for 32 bits system, this patch
> > might need to be reverted.
> >
> > @Jesper, @Ilias, what do you think?
>
>
> So enabling LPAE also enables CONFIG_ARCH_DMA_ADDR_T_64BIT on that board?
> Doing a quick grep only selects that for XEN. I am ok reverting that, but
> I think we need to understand how the dma address ended up being 64bit.
So looking a bit closer, indeed enabling LPAE always enables this. So
we need to revert the patch.
Yunsheng will you send that?
Thanks
/Ilias
>
> Regards
> /Ilias
>
> >
> > >
> > > Best wishes,
> > > Guillaume
> > >
> > >
> > >>> GitHub: https://github.com/kernelci/kernelci-project/issues/71
> > >>>
> > >>> -------------------------------------------------------------------------------
> > >>>
> > >>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> > >>> * This automated bisection report was sent to you on the basis *
> > >>> * that you may be involved with the breaking commit it has *
> > >>> * found. No manual investigation has been done to verify it, *
> > >>> * and the root cause of the problem may be somewhere else. *
> > >>> * *
> > >>> * If you do send a fix, please include this trailer: *
> > >>> * Reported-by: "kernelci.org bot" <[email protected]> *
> > >>> * *
> > >>> * Hope this helps! *
> > >>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> > >>>
> > >>> mainline/master bisection: baseline.login on rk3288-rock2-square
> > >>>
> > >>> Summary:
> > >>> Start: e851dfae4371d Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
> > >>> Plain log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.txt
> > >>> HTML log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.html
> > >>> Result: d00e60ee54b12 page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> > >>>
> > >>> Checks:
> > >>> revert: PASS
> > >>> verify: PASS
> > >>>
> > >>> Parameters:
> > >>> Tree: mainline
> > >>> URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > >>> Branch: master
> > >>> Target: rk3288-rock2-square
> > >>> CPU arch: arm
> > >>> Lab: lab-collabora
> > >>> Compiler: gcc-10
> > >>> Config: multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y
> > >>> Test case: baseline.login
> > >>>
> > >>> Breaking commit found:
> > >>>
> > >>> -------------------------------------------------------------------------------
> > >>> commit d00e60ee54b12de945b8493cf18c1ada9e422514
> > >>> Author: Yunsheng Lin <[email protected]>
> > >>> Date: Wed Oct 13 17:19:20 2021 +0800
> > >>>
> > >>> page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> > >>>
> > >>>
> > >>> On 13/10/2021 10:19, Yunsheng Lin wrote:
> > >>>> As the 32-bit arch with 64-bit DMA seems to rare those days,
> > >>>> and page pool might carry a lot of code and complexity for
> > >>>> systems that possibly.
> > >>>>
> > >>>> So disable dma mapping support for such systems, if drivers
> > >>>> really want to work on such systems, they have to implement
> > >>>> their own DMA-mapping fallback tracking outside page_pool.
> > >>>>
> > >>>> Reviewed-by: Ilias Apalodimas <[email protected]>
> > >>>> Signed-off-by: Yunsheng Lin <[email protected]>
> > >>>> ---
> > >>>> V6: Drop pp page tracking support
> > >>>> ---
> > >>>> include/linux/mm_types.h | 13 +------------
> > >>>> include/net/page_pool.h | 12 +-----------
> > >>>> net/core/page_pool.c | 10 ++++++----
> > >>>> 3 files changed, 8 insertions(+), 27 deletions(-)
> > >>>>
> > >>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > >>>> index 7f8ee09c711f..436e0946d691 100644
> > >>>> --- a/include/linux/mm_types.h
> > >>>> +++ b/include/linux/mm_types.h
> > >>>> @@ -104,18 +104,7 @@ struct page {
> > >>>> struct page_pool *pp;
> > >>>> unsigned long _pp_mapping_pad;
> > >>>> unsigned long dma_addr;
> > >>>> - union {
> > >>>> - /**
> > >>>> - * dma_addr_upper: might require a 64-bit
> > >>>> - * value on 32-bit architectures.
> > >>>> - */
> > >>>> - unsigned long dma_addr_upper;
> > >>>> - /**
> > >>>> - * For frag page support, not supported in
> > >>>> - * 32-bit architectures with 64-bit DMA.
> > >>>> - */
> > >>>> - atomic_long_t pp_frag_count;
> > >>>> - };
> > >>>> + atomic_long_t pp_frag_count;
> > >>>> };
> > >>>> struct { /* slab, slob and slub */
> > >>>> union {
> > >>>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> > >>>> index a4082406a003..3855f069627f 100644
> > >>>> --- a/include/net/page_pool.h
> > >>>> +++ b/include/net/page_pool.h
> > >>>> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
> > >>>> page_pool_put_full_page(pool, page, true);
> > >>>> }
> > >>>>
> > >>>> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
> > >>>> - (sizeof(dma_addr_t) > sizeof(unsigned long))
> > >>>> -
> > >>>> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> > >>>> {
> > >>>> - dma_addr_t ret = page->dma_addr;
> > >>>> -
> > >>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> > >>>> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
> > >>>> -
> > >>>> - return ret;
> > >>>> + return page->dma_addr;
> > >>>> }
> > >>>>
> > >>>> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
> > >>>> {
> > >>>> page->dma_addr = addr;
> > >>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
> > >>>> - page->dma_addr_upper = upper_32_bits(addr);
> > >>>> }
> > >>>>
> > >>>> static inline void page_pool_set_frag_count(struct page *page, long nr)
> > >>>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> > >>>> index 1a6978427d6c..9b60e4301a44 100644
> > >>>> --- a/net/core/page_pool.c
> > >>>> +++ b/net/core/page_pool.c
> > >>>> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
> > >>>> * which is the XDP_TX use-case.
> > >>>> */
> > >>>> if (pool->p.flags & PP_FLAG_DMA_MAP) {
> > >>>> + /* DMA-mapping is not supported on 32-bit systems with
> > >>>> + * 64-bit DMA mapping.
> > >>>> + */
> > >>>> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
> > >>>> + return -EOPNOTSUPP;
> > >>>> +
> > >>>> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
> > >>>> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
> > >>>> return -EINVAL;
> > >>>> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
> > >>>> */
> > >>>> }
> > >>>>
> > >>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
> > >>>> - pool->p.flags & PP_FLAG_PAGE_FRAG)
> > >>>> - return -EINVAL;
> > >>>> -
> > >>>> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
> > >>>> return -ENOMEM;
> > >>>>
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>> Git bisection log:
> > >>>
> > >>> -------------------------------------------------------------------------------
> > >>> git bisect start
> > >>> # good: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
> > >>> git bisect good bfc484fe6abba4b89ec9330e0e68778e2a9856b2
> > >>> # bad: [e851dfae4371d3c751f1e18e8eb5eba993de1467] Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
> > >>> git bisect bad e851dfae4371d3c751f1e18e8eb5eba993de1467
> > >>> # bad: [dcd68326d29b62f3039e4f4d23d3e38f24d37360] Merge tag 'devicetree-for-5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
> > >>> git bisect bad dcd68326d29b62f3039e4f4d23d3e38f24d37360
> > >>> # bad: [b7b98f868987cd3e86c9bd9a6db048614933d7a0] Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
> > >>> git bisect bad b7b98f868987cd3e86c9bd9a6db048614933d7a0
> > >>> # bad: [9fd3d5dced976640f588e0a866b9611db2d2cb37] net: ethernet: ave: Add compatible string and SoC-dependent data for NX1 SoC
> > >>> git bisect bad 9fd3d5dced976640f588e0a866b9611db2d2cb37
> > >>> # good: [a96d317fb1a30b9f323548eb2ff05d4e4600ead9] ethernet: use eth_hw_addr_set()
> > >>> git bisect good a96d317fb1a30b9f323548eb2ff05d4e4600ead9
> > >>> # good: [f5396b8a663f7a78ee5b75a47ee524b40795b265] ice: switchdev slow path
> > >>> git bisect good f5396b8a663f7a78ee5b75a47ee524b40795b265
> > >>> # good: [20c3d9e45ba630a7156d682a40988c0e96be1b92] hamradio: use dev_addr_set() for setting device address
> > >>> git bisect good 20c3d9e45ba630a7156d682a40988c0e96be1b92
> > >>> # bad: [a64b442137669c9e839c6a70965989b01b1253b7] net: dpaa2: add support for manual setup of IRQ coalesing
> > >>> git bisect bad a64b442137669c9e839c6a70965989b01b1253b7
> > >>> # good: [30fc7efa38f21afa48b0be6bf2053e4c10ae2c78] net, neigh: Reject creating NUD_PERMANENT with NTF_MANAGED entries
> > >>> git bisect good 30fc7efa38f21afa48b0be6bf2053e4c10ae2c78
> > >>> # bad: [13ad5ccc093ff448b99ac7e138e91e78796adb48] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
> > >>> git bisect bad 13ad5ccc093ff448b99ac7e138e91e78796adb48
> > >>> # good: [40088915f547b52635f022c1e1e18df65ae3153a] Merge branch 'octeontx2-af-miscellaneous-changes-for-cpt'
> > >>> git bisect good 40088915f547b52635f022c1e1e18df65ae3153a
> > >>> # bad: [fdbf35df9c091db9c46e57e9938e3f7a4f603a7c] dt-bindings: net: dsa: qca8k: Add SGMII clock phase properties
> > >>> git bisect bad fdbf35df9c091db9c46e57e9938e3f7a4f603a7c
> > >>> # bad: [bacc8daf97d4199316328a5d18eeafbe447143c5] xen-netback: Remove redundant initialization of variable err
> > >>> git bisect bad bacc8daf97d4199316328a5d18eeafbe447143c5
> > >>> # bad: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> > >>> git bisect bad d00e60ee54b12de945b8493cf18c1ada9e422514
> > >>> # first bad commit: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
> > >>> -------------------------------------------------------------------------------
> > >>> .
> > >>>
> > >
> > > .
> > >
On 2021/11/15 20:10, Ilias Apalodimas wrote:
> On Mon, 15 Nov 2021 at 13:53, Ilias Apalodimas
> <[email protected]> wrote:
>>
>> On Mon, Nov 15, 2021 at 11:34:59AM +0800, Yunsheng Lin wrote:
>>> On 2021/11/12 17:21, Guillaume Tucker wrote:
>>>> On 09/11/2021 12:02, Yunsheng Lin wrote:
>>>>> On 2021/11/9 17:58, Guillaume Tucker wrote:
>>>>>> Hi Yunsheng,
>>>>>>
>>>>>> Please see the bisection report below about a boot failure on
>>>>>> rk3288-rock2-square which is pointing to this patch. The issue
>>>>>> appears to only happen with CONFIG_ARM_LPAE=y.
>>>>>>
>>>>>> Reports aren't automatically sent to the public while we're
>>>>>> trialing new bisection features on kernelci.org but this one
>>>>>> looks valid.
>>>>>>
>>>>>> Some more details can be found here:
>>>>>>
>>>>>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
>>>>>>
>>>>>> Here's the same revision on the same platform booting fine with a
>>>>>> plain multi_v7_defconfig build:
>>>>>>
>>>>>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
>>>>>>
>>>>>> Please let us know if you need any help debugging this issue or
>>>>>> if you have a fix to try.
>>>>>
>>>>> The patch below is removing the dma mapping support in page pool
>>>>> for 32 bit systems with 64 bit dma address, so it seems there
>>>>> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
>>>>> flags set in a 32 bit systems with 64 bit dma address.
>>>>>
>>>>> It seems we might need to revert the below patch or implement the
>>>>> DMA-mapping tracking support in the driver as mentioned in the below
>>>>> commit log.
>>>>>
>>>>> which ethernet driver do you use in your system?
>>>>
>>>> Thanks for taking a look and sorry for the slow reply. Here's a
>>>> booting test job with LPAE disabled:
>>>>
>>>> https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
>>>> https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
>>>>
>>>> [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>>>>
>>>> So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>
>>> Thanks for the report, this patch seems to cause problem for 32-bit
>>> system with LPAE enabled.
>>>
>>> As LPAE seems like a common feature for 32 bits system, this patch
>>> might need to be reverted.
>>>
>>> @Jesper, @Ilias, what do you think?
>>
>>
>> So enabling LPAE also enables CONFIG_ARCH_DMA_ADDR_T_64BIT on that board?
>> Doing a quick grep only selects that for XEN. I am ok reverting that, but
>> I think we need to understand how the dma address ended up being 64bit.
>
> So looking a bit closer, indeed enabling LPAE always enables this. So
> we need to revert the patch.
> Yunsheng will you send that?
Sure.
>
> Thanks
> /Ilias
>>
>> Regards
>> /Ilias
>>
>>>
>>>>
>>>> Best wishes,
>>>> Guillaume
>>>>
>>>>
>>>>>> GitHub: https://github.com/kernelci/kernelci-project/issues/71
>>>>>>
>>>>>> -------------------------------------------------------------------------------
>>>>>>
>>>>>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>>>>>> * This automated bisection report was sent to you on the basis *
>>>>>> * that you may be involved with the breaking commit it has *
>>>>>> * found. No manual investigation has been done to verify it, *
>>>>>> * and the root cause of the problem may be somewhere else. *
>>>>>> * *
>>>>>> * If you do send a fix, please include this trailer: *
>>>>>> * Reported-by: "kernelci.org bot" <[email protected]> *
>>>>>> * *
>>>>>> * Hope this helps! *
>>>>>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>>>>>>
>>>>>> mainline/master bisection: baseline.login on rk3288-rock2-square
>>>>>>
>>>>>> Summary:
>>>>>> Start: e851dfae4371d Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
>>>>>> Plain log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.txt
>>>>>> HTML log: https://storage.kernelci.org/mainline/master/v5.15-11387-ge851dfae4371/arm/multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y/gcc-10/lab-collabora/baseline-rk3288-rock2-square.html
>>>>>> Result: d00e60ee54b12 page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>>>>>
>>>>>> Checks:
>>>>>> revert: PASS
>>>>>> verify: PASS
>>>>>>
>>>>>> Parameters:
>>>>>> Tree: mainline
>>>>>> URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>>>>> Branch: master
>>>>>> Target: rk3288-rock2-square
>>>>>> CPU arch: arm
>>>>>> Lab: lab-collabora
>>>>>> Compiler: gcc-10
>>>>>> Config: multi_v7_defconfig+CONFIG_EFI=y+CONFIG_ARM_LPAE=y
>>>>>> Test case: baseline.login
>>>>>>
>>>>>> Breaking commit found:
>>>>>>
>>>>>> -------------------------------------------------------------------------------
>>>>>> commit d00e60ee54b12de945b8493cf18c1ada9e422514
>>>>>> Author: Yunsheng Lin <[email protected]>
>>>>>> Date: Wed Oct 13 17:19:20 2021 +0800
>>>>>>
>>>>>> page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>>>>>
>>>>>>
>>>>>> On 13/10/2021 10:19, Yunsheng Lin wrote:
>>>>>>> As the 32-bit arch with 64-bit DMA seems to rare those days,
>>>>>>> and page pool might carry a lot of code and complexity for
>>>>>>> systems that possibly.
>>>>>>>
>>>>>>> So disable dma mapping support for such systems, if drivers
>>>>>>> really want to work on such systems, they have to implement
>>>>>>> their own DMA-mapping fallback tracking outside page_pool.
>>>>>>>
>>>>>>> Reviewed-by: Ilias Apalodimas <[email protected]>
>>>>>>> Signed-off-by: Yunsheng Lin <[email protected]>
>>>>>>> ---
>>>>>>> V6: Drop pp page tracking support
>>>>>>> ---
>>>>>>> include/linux/mm_types.h | 13 +------------
>>>>>>> include/net/page_pool.h | 12 +-----------
>>>>>>> net/core/page_pool.c | 10 ++++++----
>>>>>>> 3 files changed, 8 insertions(+), 27 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>>>>>>> index 7f8ee09c711f..436e0946d691 100644
>>>>>>> --- a/include/linux/mm_types.h
>>>>>>> +++ b/include/linux/mm_types.h
>>>>>>> @@ -104,18 +104,7 @@ struct page {
>>>>>>> struct page_pool *pp;
>>>>>>> unsigned long _pp_mapping_pad;
>>>>>>> unsigned long dma_addr;
>>>>>>> - union {
>>>>>>> - /**
>>>>>>> - * dma_addr_upper: might require a 64-bit
>>>>>>> - * value on 32-bit architectures.
>>>>>>> - */
>>>>>>> - unsigned long dma_addr_upper;
>>>>>>> - /**
>>>>>>> - * For frag page support, not supported in
>>>>>>> - * 32-bit architectures with 64-bit DMA.
>>>>>>> - */
>>>>>>> - atomic_long_t pp_frag_count;
>>>>>>> - };
>>>>>>> + atomic_long_t pp_frag_count;
>>>>>>> };
>>>>>>> struct { /* slab, slob and slub */
>>>>>>> union {
>>>>>>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>>>>>>> index a4082406a003..3855f069627f 100644
>>>>>>> --- a/include/net/page_pool.h
>>>>>>> +++ b/include/net/page_pool.h
>>>>>>> @@ -216,24 +216,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>>>>>>> page_pool_put_full_page(pool, page, true);
>>>>>>> }
>>>>>>>
>>>>>>> -#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
>>>>>>> - (sizeof(dma_addr_t) > sizeof(unsigned long))
>>>>>>> -
>>>>>>> static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>>>>>>> {
>>>>>>> - dma_addr_t ret = page->dma_addr;
>>>>>>> -
>>>>>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>>>>>>> - ret |= (dma_addr_t)page->dma_addr_upper << 16 << 16;
>>>>>>> -
>>>>>>> - return ret;
>>>>>>> + return page->dma_addr;
>>>>>>> }
>>>>>>>
>>>>>>> static inline void page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
>>>>>>> {
>>>>>>> page->dma_addr = addr;
>>>>>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT)
>>>>>>> - page->dma_addr_upper = upper_32_bits(addr);
>>>>>>> }
>>>>>>>
>>>>>>> static inline void page_pool_set_frag_count(struct page *page, long nr)
>>>>>>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>>>>>>> index 1a6978427d6c..9b60e4301a44 100644
>>>>>>> --- a/net/core/page_pool.c
>>>>>>> +++ b/net/core/page_pool.c
>>>>>>> @@ -49,6 +49,12 @@ static int page_pool_init(struct page_pool *pool,
>>>>>>> * which is the XDP_TX use-case.
>>>>>>> */
>>>>>>> if (pool->p.flags & PP_FLAG_DMA_MAP) {
>>>>>>> + /* DMA-mapping is not supported on 32-bit systems with
>>>>>>> + * 64-bit DMA mapping.
>>>>>>> + */
>>>>>>> + if (sizeof(dma_addr_t) > sizeof(unsigned long))
>>>>>>> + return -EOPNOTSUPP;
>>>>>>> +
>>>>>>> if ((pool->p.dma_dir != DMA_FROM_DEVICE) &&
>>>>>>> (pool->p.dma_dir != DMA_BIDIRECTIONAL))
>>>>>>> return -EINVAL;
>>>>>>> @@ -69,10 +75,6 @@ static int page_pool_init(struct page_pool *pool,
>>>>>>> */
>>>>>>> }
>>>>>>>
>>>>>>> - if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
>>>>>>> - pool->p.flags & PP_FLAG_PAGE_FRAG)
>>>>>>> - return -EINVAL;
>>>>>>> -
>>>>>>> if (ptr_ring_init(&pool->ring, ring_qsize, GFP_KERNEL) < 0)
>>>>>>> return -ENOMEM;
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Git bisection log:
>>>>>>
>>>>>> -------------------------------------------------------------------------------
>>>>>> git bisect start
>>>>>> # good: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
>>>>>> git bisect good bfc484fe6abba4b89ec9330e0e68778e2a9856b2
>>>>>> # bad: [e851dfae4371d3c751f1e18e8eb5eba993de1467] Merge tag 'kgdb-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux
>>>>>> git bisect bad e851dfae4371d3c751f1e18e8eb5eba993de1467
>>>>>> # bad: [dcd68326d29b62f3039e4f4d23d3e38f24d37360] Merge tag 'devicetree-for-5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
>>>>>> git bisect bad dcd68326d29b62f3039e4f4d23d3e38f24d37360
>>>>>> # bad: [b7b98f868987cd3e86c9bd9a6db048614933d7a0] Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
>>>>>> git bisect bad b7b98f868987cd3e86c9bd9a6db048614933d7a0
>>>>>> # bad: [9fd3d5dced976640f588e0a866b9611db2d2cb37] net: ethernet: ave: Add compatible string and SoC-dependent data for NX1 SoC
>>>>>> git bisect bad 9fd3d5dced976640f588e0a866b9611db2d2cb37
>>>>>> # good: [a96d317fb1a30b9f323548eb2ff05d4e4600ead9] ethernet: use eth_hw_addr_set()
>>>>>> git bisect good a96d317fb1a30b9f323548eb2ff05d4e4600ead9
>>>>>> # good: [f5396b8a663f7a78ee5b75a47ee524b40795b265] ice: switchdev slow path
>>>>>> git bisect good f5396b8a663f7a78ee5b75a47ee524b40795b265
>>>>>> # good: [20c3d9e45ba630a7156d682a40988c0e96be1b92] hamradio: use dev_addr_set() for setting device address
>>>>>> git bisect good 20c3d9e45ba630a7156d682a40988c0e96be1b92
>>>>>> # bad: [a64b442137669c9e839c6a70965989b01b1253b7] net: dpaa2: add support for manual setup of IRQ coalesing
>>>>>> git bisect bad a64b442137669c9e839c6a70965989b01b1253b7
>>>>>> # good: [30fc7efa38f21afa48b0be6bf2053e4c10ae2c78] net, neigh: Reject creating NUD_PERMANENT with NTF_MANAGED entries
>>>>>> git bisect good 30fc7efa38f21afa48b0be6bf2053e4c10ae2c78
>>>>>> # bad: [13ad5ccc093ff448b99ac7e138e91e78796adb48] dt-bindings: net: dsa: qca8k: Document qca,sgmii-enable-pll
>>>>>> git bisect bad 13ad5ccc093ff448b99ac7e138e91e78796adb48
>>>>>> # good: [40088915f547b52635f022c1e1e18df65ae3153a] Merge branch 'octeontx2-af-miscellaneous-changes-for-cpt'
>>>>>> git bisect good 40088915f547b52635f022c1e1e18df65ae3153a
>>>>>> # bad: [fdbf35df9c091db9c46e57e9938e3f7a4f603a7c] dt-bindings: net: dsa: qca8k: Add SGMII clock phase properties
>>>>>> git bisect bad fdbf35df9c091db9c46e57e9938e3f7a4f603a7c
>>>>>> # bad: [bacc8daf97d4199316328a5d18eeafbe447143c5] xen-netback: Remove redundant initialization of variable err
>>>>>> git bisect bad bacc8daf97d4199316328a5d18eeafbe447143c5
>>>>>> # bad: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>>>>> git bisect bad d00e60ee54b12de945b8493cf18c1ada9e422514
>>>>>> # first bad commit: [d00e60ee54b12de945b8493cf18c1ada9e422514] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA
>>>>>> -------------------------------------------------------------------------------
>>>>>> .
>>>>>>
>>>>
>>>> .
>>>>
> .
>
On 15/11/2021 13.21, Yunsheng Lin wrote:
> On 2021/11/15 20:10, Ilias Apalodimas wrote:
>> On Mon, 15 Nov 2021 at 13:53, Ilias Apalodimas
>> <[email protected]> wrote:
>>>
>>> On Mon, Nov 15, 2021 at 11:34:59AM +0800, Yunsheng Lin wrote:
>>>> On 2021/11/12 17:21, Guillaume Tucker wrote:
>>>>> On 09/11/2021 12:02, Yunsheng Lin wrote:
>>>>>> On 2021/11/9 17:58, Guillaume Tucker wrote:
>>>>>>> Hi Yunsheng,
>>>>>>>
>>>>>>> Please see the bisection report below about a boot failure on
>>>>>>> rk3288-rock2-square which is pointing to this patch. The issue
>>>>>>> appears to only happen with CONFIG_ARM_LPAE=y.
>>>>>>>
>>>>>>> Reports aren't automatically sent to the public while we're
>>>>>>> trialing new bisection features on kernelci.org but this one
>>>>>>> looks valid.
>>>>>>>
>>>>>>> Some more details can be found here:
>>>>>>>
>>>>>>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
>>>>>>>
>>>>>>> Here's the same revision on the same platform booting fine with a
>>>>>>> plain multi_v7_defconfig build:
>>>>>>>
>>>>>>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
>>>>>>>
>>>>>>> Please let us know if you need any help debugging this issue or
>>>>>>> if you have a fix to try.
>>>>>>
>>>>>> The patch below is removing the dma mapping support in page pool
>>>>>> for 32 bit systems with 64 bit dma address, so it seems there
>>>>>> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
>>>>>> flags set in a 32 bit systems with 64 bit dma address.
>>>>>>
>>>>>> It seems we might need to revert the below patch or implement the
>>>>>> DMA-mapping tracking support in the driver as mentioned in the below
>>>>>> commit log.
>>>>>>
>>>>>> which ethernet driver do you use in your system?
>>>>>
>>>>> Thanks for taking a look and sorry for the slow reply. Here's a
>>>>> booting test job with LPAE disabled:
>>>>>
>>>>> https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
>>>>> https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
>>>>>
>>>>> [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>>>>>
>>>>> So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>
>>>> Thanks for the report, this patch seems to cause problem for 32-bit
>>>> system with LPAE enabled.
>>>>
>>>> As LPAE seems like a common feature for 32 bits system, this patch
>>>> might need to be reverted.
>>>>
>>>> @Jesper, @Ilias, what do you think?
>>>
>>>
>>> So enabling LPAE also enables CONFIG_ARCH_DMA_ADDR_T_64BIT on that board?
>>> Doing a quick grep only selects that for XEN. I am ok reverting that, but
>>> I think we need to understand how the dma address ended up being 64bit.
>>
>> So looking a bit closer, indeed enabling LPAE always enables this. So
>> we need to revert the patch.
>> Yunsheng will you send that?
>
> Sure.
Why don't we change that driver[1] to not use page_pool_get_dma_addr() ?
[1] drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
I took a closer look and it seems the driver have struct
stmmac_rx_buffer in which is stored the dma_addr it gets from
page_pool_get_dma_addr().
See func: stmmac_init_rx_buffers
static int stmmac_init_rx_buffers(struct stmmac_priv *priv,
struct dma_desc *p,
int i, gfp_t flags, u32 queue)
{
if (!buf->page) {
buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
if (!buf->page)
return -ENOMEM;
buf->page_offset = stmmac_rx_offset(priv);
}
[...]
buf->addr = page_pool_get_dma_addr(buf->page) + buf->page_offset;
stmmac_set_desc_addr(priv, p, buf->addr);
[...]
}
I question if this driver really to use page_pool for storing the
dma_addr as it just extract it and store it outside page_pool?
@Ilias it looks like you added part of the page_pool support in this
driver, so I hope you can give a qualified guess on:
How much work will it be to let driver do the DMA-map itself?
(and not depend on the DMA-map feature provided by page_pool)
--Jesper
This is just popping out of nowhere on lkml, but yes ARM LPAE
uses a 64-bit dma_addr_t, as a 64-bit phys_addr_t implies that.
Same for x86-32 PAE and similar cases on MIPS and probably a few
other architectures.
But what is the actual problem with a 32-bit virtual and 64-bit
pysical/dma address? That is a pretty common setup and absolutely
needs to be deal with in drivers and inrastructure.
[...]
> > > > > > > > Some more details can be found here:
> > > > > > > >
> > > > > > > > https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
> > > > > > > >
> > > > > > > > Here's the same revision on the same platform booting fine with a
> > > > > > > > plain multi_v7_defconfig build:
> > > > > > > >
> > > > > > > > https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
> > > > > > > >
> > > > > > > > Please let us know if you need any help debugging this issue or
> > > > > > > > if you have a fix to try.
> > > > > > >
> > > > > > > The patch below is removing the dma mapping support in page pool
> > > > > > > for 32 bit systems with 64 bit dma address, so it seems there
> > > > > > > is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
> > > > > > > flags set in a 32 bit systems with 64 bit dma address.
> > > > > > >
> > > > > > > It seems we might need to revert the below patch or implement the
> > > > > > > DMA-mapping tracking support in the driver as mentioned in the below
> > > > > > > commit log.
> > > > > > >
> > > > > > > which ethernet driver do you use in your system?
> > > > > >
> > > > > > Thanks for taking a look and sorry for the slow reply. Here's a
> > > > > > booting test job with LPAE disabled:
> > > > > >
> > > > > > https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
> > > > > > https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
> > > > > >
> > > > > > [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> > > > > >
> > > > > > So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > >
> > > > > Thanks for the report, this patch seems to cause problem for 32-bit
> > > > > system with LPAE enabled.
> > > > >
> > > > > As LPAE seems like a common feature for 32 bits system, this patch
> > > > > might need to be reverted.
> > > > >
> > > > > @Jesper, @Ilias, what do you think?
> > > >
> > > >
> > > > So enabling LPAE also enables CONFIG_ARCH_DMA_ADDR_T_64BIT on that board?
> > > > Doing a quick grep only selects that for XEN. I am ok reverting that, but
> > > > I think we need to understand how the dma address ended up being 64bit.
> > >
> > > So looking a bit closer, indeed enabling LPAE always enables this. So
> > > we need to revert the patch.
> > > Yunsheng will you send that?
> >
> > Sure.
>
> Why don't we change that driver[1] to not use page_pool_get_dma_addr() ?
>
> [1] drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>
> I took a closer look and it seems the driver have struct stmmac_rx_buffer in
> which is stored the dma_addr it gets from page_pool_get_dma_addr().
>
> See func: stmmac_init_rx_buffers
>
> static int stmmac_init_rx_buffers(struct stmmac_priv *priv,
> struct dma_desc *p,
> int i, gfp_t flags, u32 queue)
> {
>
> if (!buf->page) {
> buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
> if (!buf->page)
> return -ENOMEM;
> buf->page_offset = stmmac_rx_offset(priv);
> }
> [...]
>
> buf->addr = page_pool_get_dma_addr(buf->page) + buf->page_offset;
>
> stmmac_set_desc_addr(priv, p, buf->addr);
> [...]
> }
>
> I question if this driver really to use page_pool for storing the dma_addr
> as it just extract it and store it outside page_pool?
>
> @Ilias it looks like you added part of the page_pool support in this driver,
> so I hope you can give a qualified guess on:
> How much work will it be to let driver do the DMA-map itself?
> (and not depend on the DMA-map feature provided by page_pool)
It shouldn't be that hard. However when we removed that we were hoping we
had no active consumers. So we'll have to fix this and check for other
32-bit boards with LPAE and page_pool handling the DMA mappings.
But the point now is that this is far from a 'hardware configuration' of
32-bit CPU + 64-bit DMA. Every armv7 and x86 board can get that. So I was
thinking it's better to revert this and live with the 'weird' handling in the
code.
Cheers
/Ilias
On Mon, Nov 15, 2021 at 08:48:30PM +0200, Ilias Apalodimas wrote:
> page_pool (the API in question), apart from allocating memory can manage
> the mappings for you. However while doing so it stores some parts (incl
> the dma addr) in struct page. The code in there could be simplified if
> we skipped support of the 'mapping' feature for 32-bit architectures with
> 64-bit DMA. We thought no driver was using the mapping feature (on 32bits)
> and cleaned up that part, but apparently we missed
> '32-bit -- LPAE -- page pool manages DMA mappings'
It is a very common configuration on various architectures, so I fear
you'll have to support it and undo the cleanup.
Hi Christoph,
On Mon, Nov 15, 2021 at 09:59:50AM -0800, Christoph Hellwig wrote:
> This is just popping out of nowhere on lkml, but yes ARM LPAE
> uses a 64-bit dma_addr_t, as a 64-bit phys_addr_t implies that.
> Same for x86-32 PAE and similar cases on MIPS and probably a few
> other architectures.
>
> But what is the actual problem with a 32-bit virtual and 64-bit
> pysical/dma address? That is a pretty common setup and absolutely
> needs to be deal with in drivers and inrastructure.
page_pool (the API in question), apart from allocating memory can manage
the mappings for you. However while doing so it stores some parts (incl
the dma addr) in struct page. The code in there could be simplified if
we skipped support of the 'mapping' feature for 32-bit architectures with
64-bit DMA. We thought no driver was using the mapping feature (on 32bits)
and cleaned up that part, but apparently we missed
'32-bit -- LPAE -- page pool manages DMA mappings'
Regards
/Ilias
On 15/11/2021 19.55, Ilias Apalodimas wrote:
>
> [...]
>
>>>>>>>>> Some more details can be found here:
>>>>>>>>>
>>>>>>>>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/
>>>>>>>>>
>>>>>>>>> Here's the same revision on the same platform booting fine with a
>>>>>>>>> plain multi_v7_defconfig build:
>>>>>>>>>
>>>>>>>>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/
>>>>>>>>>
>>>>>>>>> Please let us know if you need any help debugging this issue or
>>>>>>>>> if you have a fix to try.
>>>>>>>>
>>>>>>>> The patch below is removing the dma mapping support in page pool
>>>>>>>> for 32 bit systems with 64 bit dma address, so it seems there
>>>>>>>> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP
>>>>>>>> flags set in a 32 bit systems with 64 bit dma address.
>>>>>>>>
>>>>>>>> It seems we might need to revert the below patch or implement the
>>>>>>>> DMA-mapping tracking support in the driver as mentioned in the below
>>>>>>>> commit log.
>>>>>>>>
>>>>>>>> which ethernet driver do you use in your system?
>>>>>>>
>>>>>>> Thanks for taking a look and sorry for the slow reply. Here's a
>>>>>>> booting test job with LPAE disabled:
>>>>>>>
>>>>>>> https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/
>>>>>>> https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812
>>>>>>>
>>>>>>> [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>>>>>>>
>>>>>>> So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>
>>>>>> Thanks for the report, this patch seems to cause problem for 32-bit
>>>>>> system with LPAE enabled.
>>>>>>
>>>>>> As LPAE seems like a common feature for 32 bits system, this patch
>>>>>> might need to be reverted.
>>>>>>
>>>>>> @Jesper, @Ilias, what do you think?
>>>>>
>>>>>
>>>>> So enabling LPAE also enables CONFIG_ARCH_DMA_ADDR_T_64BIT on that board?
>>>>> Doing a quick grep only selects that for XEN. I am ok reverting that, but
>>>>> I think we need to understand how the dma address ended up being 64bit.
>>>>
>>>> So looking a bit closer, indeed enabling LPAE always enables this. So
>>>> we need to revert the patch.
>>>> Yunsheng will you send that?
>>>
>>> Sure.
>>
>> Why don't we change that driver[1] to not use page_pool_get_dma_addr() ?
>>
>> [1] drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>
>> I took a closer look and it seems the driver have struct stmmac_rx_buffer in
>> which is stored the dma_addr it gets from page_pool_get_dma_addr().
>>
>> See func: stmmac_init_rx_buffers
>>
>> static int stmmac_init_rx_buffers(struct stmmac_priv *priv,
>> struct dma_desc *p,
>> int i, gfp_t flags, u32 queue)
>> {
>>
>> if (!buf->page) {
>> buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
>> if (!buf->page)
>> return -ENOMEM;
>> buf->page_offset = stmmac_rx_offset(priv);
>> }
>> [...]
>>
>> buf->addr = page_pool_get_dma_addr(buf->page) + buf->page_offset;
>>
>> stmmac_set_desc_addr(priv, p, buf->addr);
>> [...]
>> }
>>
>> I question if this driver really to use page_pool for storing the dma_addr
>> as it just extract it and store it outside page_pool?
>>
>> @Ilias it looks like you added part of the page_pool support in this driver,
>> so I hope you can give a qualified guess on:
>> How much work will it be to let driver do the DMA-map itself?
>> (and not depend on the DMA-map feature provided by page_pool)
>
> It shouldn't be that hard. However when we removed that we were hoping we
> had no active consumers. So we'll have to fix this and check for other
> 32-bit boards with LPAE and page_pool handling the DMA mappings.
> But the point now is that this is far from a 'hardware configuration' of
> 32-bit CPU + 64-bit DMA. Every armv7 and x86 board can get that. So I was
> thinking it's better to revert this and live with the 'weird' handling in the
> code.
Okay, I acked the revert. After discussing this over IRC with Ilias (my
page_pool co-maintainer). Guess we will have to live with maintaining
this code for 32-bit CPU + 64-bit DMA.
--Jesper