* struct dev_pagemap corruption
@ 2019-04-05 4:40 ` Anshuman Khandual
0 siblings, 0 replies; 16+ messages in thread
From: Anshuman Khandual @ 2019-04-05 4:40 UTC (permalink / raw)
To: linux-mm, Linux ARM, Jérôme Glisse, Dan Williams,
Will Deacon, Catalin Marinas
Hello,
On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
unmapping path through device_destroy(). Its device memory range end address
(pgmap->res.end) which is getting corrupted in this particular case. AFAICS
pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
should retain it's values during the unmapping path as well. Is this assumption
right ?
[ 62.779412] Call trace:
[ 62.779808] dump_backtrace+0x0/0x118
[ 62.780460] show_stack+0x14/0x20
[ 62.781204] dump_stack+0xa8/0xcc
[ 62.781941] devm_memremap_pages_release+0x24/0x1d8
[ 62.783021] devm_action_release+0x10/0x18
[ 62.783911] release_nodes+0x1b0/0x220
[ 62.784732] devres_release_all+0x34/0x50
[ 62.785623] device_release+0x24/0x90
[ 62.786454] kobject_put+0x74/0xe8
[ 62.787214] device_destroy+0x48/0x58
[ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
[ 62.789675] do_one_initcall+0x74/0x190
[ 62.790528] do_init_module+0x50/0x1c0
[ 62.791346] load_module+0x1be4/0x2140
[ 62.792192] __se_sys_finit_module+0xb8/0xc8
[ 62.793128] __arm64_sys_finit_module+0x18/0x20
[ 62.794128] el0_svc_handler+0x88/0x100
[ 62.794989] el0_svc+0x8/0xc
The problem can be traced down here.
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index e038e2b3b7ea..2a410c88c596 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -33,7 +33,7 @@ struct devres {
* Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
* buffer alignment as if it was allocated by plain kmalloc().
*/
- u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
+ u8 __aligned(__alignof__(unsigned long long)) data[];
};
On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
dev_pagemap being added:
#define ZONE_DEVICE_PHYS_START 0x680000000UL
#define ZONE_DEVICE_PHYS_END 0x6BFFFFFFFUL
#define ALTMAP_FREE 4096
#define ALTMAP_RESV 1024
pgmap->type = MEMORY_DEVICE_PUBLIC;
pgmap->res.start = ZONE_DEVICE_PHYS_START;
pgmap->res.end = ZONE_DEVICE_PHYS_END;
pgmap->ref = ref;
pgmap->kill = zone_device_percpu_kill;
pgmap->dev = dev;
memset(&pgmap->altmap, 0, sizeof(struct vmem_altmap));
pgmap->altmap.free = ALTMAP_FREE;
pgmap->altmap.alloc = 0;
pgmap->altmap.align = 0;
pgmap->altmap_valid = 1;
tmp = (unsigned long *)&pgmap->altmap.base_pfn;
tmp1 = (unsigned long *)&pgmap->altmap.reserve;
*tmp = pgmap->res.start >> PAGE_SHIFT;
*tmp1 = ALTMAP_RESV;
With the patch:
[ 53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
[ 53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
Without the patch:
[ 34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
[ 34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
Though this prevents the above corruption I wonder what was causing it in the
first place and how we can address the problem.
- Anshuman
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
2019-04-05 4:40 ` Anshuman Khandual
@ 2019-04-05 13:37 ` Robin Murphy
-1 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2019-04-05 13:37 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, Linux ARM, Jérôme Glisse,
Dan Williams, Will Deacon, Catalin Marinas
On 05/04/2019 05:40, Anshuman Khandual wrote:
> Hello,
>
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
>
> [ 62.779412] Call trace:
> [ 62.779808] dump_backtrace+0x0/0x118
> [ 62.780460] show_stack+0x14/0x20
> [ 62.781204] dump_stack+0xa8/0xcc
> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
> [ 62.783021] devm_action_release+0x10/0x18
> [ 62.783911] release_nodes+0x1b0/0x220
> [ 62.784732] devres_release_all+0x34/0x50
> [ 62.785623] device_release+0x24/0x90
> [ 62.786454] kobject_put+0x74/0xe8
> [ 62.787214] device_destroy+0x48/0x58
> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
> [ 62.789675] do_one_initcall+0x74/0x190
> [ 62.790528] do_init_module+0x50/0x1c0
> [ 62.791346] load_module+0x1be4/0x2140
> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
> [ 62.794128] el0_svc_handler+0x88/0x100
> [ 62.794989] el0_svc+0x8/0xc
>
> The problem can be traced down here.
>
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> * buffer alignment as if it was allocated by plain kmalloc().
> */
> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> + u8 __aligned(__alignof__(unsigned long long)) data[];
> };
>
> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>
> dev_pagemap being added:
>
> #define ZONE_DEVICE_PHYS_START 0x680000000UL
> #define ZONE_DEVICE_PHYS_END 0x6BFFFFFFFUL
> #define ALTMAP_FREE 4096
> #define ALTMAP_RESV 1024
>
> pgmap->type = MEMORY_DEVICE_PUBLIC;
Given that what seems to ultimately get corrupted is the memory pointed
to by pgmap here, how is *that* being allocated?
Robin.
> pgmap->res.start = ZONE_DEVICE_PHYS_START;
> pgmap->res.end = ZONE_DEVICE_PHYS_END;
> pgmap->ref = ref;
> pgmap->kill = zone_device_percpu_kill;
> pgmap->dev = dev;
>
> memset(&pgmap->altmap, 0, sizeof(struct vmem_altmap));
> pgmap->altmap.free = ALTMAP_FREE;
> pgmap->altmap.alloc = 0;
> pgmap->altmap.align = 0;
> pgmap->altmap_valid = 1;
>
> tmp = (unsigned long *)&pgmap->altmap.base_pfn;
> tmp1 = (unsigned long *)&pgmap->altmap.reserve;
>
> *tmp = pgmap->res.start >> PAGE_SHIFT;
> *tmp1 = ALTMAP_RESV;
>
> With the patch:
>
> [ 53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> [ 53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>
> Without the patch:
>
> [ 34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
> [ 34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
>
> Though this prevents the above corruption I wonder what was causing it in the
> first place and how we can address the problem.
>
> - Anshuman
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
@ 2019-04-05 13:37 ` Robin Murphy
0 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2019-04-05 13:37 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, Linux ARM, Jérôme Glisse,
Dan Williams, Will Deacon, Catalin Marinas
On 05/04/2019 05:40, Anshuman Khandual wrote:
> Hello,
>
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
>
> [ 62.779412] Call trace:
> [ 62.779808] dump_backtrace+0x0/0x118
> [ 62.780460] show_stack+0x14/0x20
> [ 62.781204] dump_stack+0xa8/0xcc
> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
> [ 62.783021] devm_action_release+0x10/0x18
> [ 62.783911] release_nodes+0x1b0/0x220
> [ 62.784732] devres_release_all+0x34/0x50
> [ 62.785623] device_release+0x24/0x90
> [ 62.786454] kobject_put+0x74/0xe8
> [ 62.787214] device_destroy+0x48/0x58
> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
> [ 62.789675] do_one_initcall+0x74/0x190
> [ 62.790528] do_init_module+0x50/0x1c0
> [ 62.791346] load_module+0x1be4/0x2140
> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
> [ 62.794128] el0_svc_handler+0x88/0x100
> [ 62.794989] el0_svc+0x8/0xc
>
> The problem can be traced down here.
>
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> * buffer alignment as if it was allocated by plain kmalloc().
> */
> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> + u8 __aligned(__alignof__(unsigned long long)) data[];
> };
>
> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>
> dev_pagemap being added:
>
> #define ZONE_DEVICE_PHYS_START 0x680000000UL
> #define ZONE_DEVICE_PHYS_END 0x6BFFFFFFFUL
> #define ALTMAP_FREE 4096
> #define ALTMAP_RESV 1024
>
> pgmap->type = MEMORY_DEVICE_PUBLIC;
Given that what seems to ultimately get corrupted is the memory pointed
to by pgmap here, how is *that* being allocated?
Robin.
> pgmap->res.start = ZONE_DEVICE_PHYS_START;
> pgmap->res.end = ZONE_DEVICE_PHYS_END;
> pgmap->ref = ref;
> pgmap->kill = zone_device_percpu_kill;
> pgmap->dev = dev;
>
> memset(&pgmap->altmap, 0, sizeof(struct vmem_altmap));
> pgmap->altmap.free = ALTMAP_FREE;
> pgmap->altmap.alloc = 0;
> pgmap->altmap.align = 0;
> pgmap->altmap_valid = 1;
>
> tmp = (unsigned long *)&pgmap->altmap.base_pfn;
> tmp1 = (unsigned long *)&pgmap->altmap.reserve;
>
> *tmp = pgmap->res.start >> PAGE_SHIFT;
> *tmp1 = ALTMAP_RESV;
>
> With the patch:
>
> [ 53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> [ 53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>
> Without the patch:
>
> [ 34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
> [ 34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
>
> Though this prevents the above corruption I wonder what was causing it in the
> first place and how we can address the problem.
>
> - Anshuman
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
2019-04-05 13:37 ` Robin Murphy
@ 2019-04-05 14:54 ` Anshuman Khandual
-1 siblings, 0 replies; 16+ messages in thread
From: Anshuman Khandual @ 2019-04-05 14:54 UTC (permalink / raw)
To: Robin Murphy, linux-mm, Linux ARM, Jérôme Glisse,
Dan Williams, Will Deacon, Catalin Marinas
On 04/05/2019 07:07 PM, Robin Murphy wrote:
> On 05/04/2019 05:40, Anshuman Khandual wrote:
>> Hello,
>>
>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>> unmapping path through device_destroy(). Its device memory range end address
>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>> should retain it's values during the unmapping path as well. Is this assumption
>> right ?
>>
>> [ 62.779412] Call trace:
>> [ 62.779808] dump_backtrace+0x0/0x118
>> [ 62.780460] show_stack+0x14/0x20
>> [ 62.781204] dump_stack+0xa8/0xcc
>> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
>> [ 62.783021] devm_action_release+0x10/0x18
>> [ 62.783911] release_nodes+0x1b0/0x220
>> [ 62.784732] devres_release_all+0x34/0x50
>> [ 62.785623] device_release+0x24/0x90
>> [ 62.786454] kobject_put+0x74/0xe8
>> [ 62.787214] device_destroy+0x48/0x58
>> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
>> [ 62.789675] do_one_initcall+0x74/0x190
>> [ 62.790528] do_init_module+0x50/0x1c0
>> [ 62.791346] load_module+0x1be4/0x2140
>> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
>> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
>> [ 62.794128] el0_svc_handler+0x88/0x100
>> [ 62.794989] el0_svc+0x8/0xc
>>
>> The problem can be traced down here.
>>
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>> index e038e2b3b7ea..2a410c88c596 100644
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -33,7 +33,7 @@ struct devres {
>> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>> * buffer alignment as if it was allocated by plain kmalloc().
>> */
>> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>> + u8 __aligned(__alignof__(unsigned long long)) data[];
>> };
>>
>> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>>
>> dev_pagemap being added:
>>
>> #define ZONE_DEVICE_PHYS_START 0x680000000UL
>> #define ZONE_DEVICE_PHYS_END 0x6BFFFFFFFUL
>> #define ALTMAP_FREE 4096
>> #define ALTMAP_RESV 1024
>>
>> pgmap->type = MEMORY_DEVICE_PUBLIC;
>
> Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
struct dev_pagemap *pgmap;
pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
functions ? kzalloc() seems to solve the problem. Will do some more testing
tomorrow.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
@ 2019-04-05 14:54 ` Anshuman Khandual
0 siblings, 0 replies; 16+ messages in thread
From: Anshuman Khandual @ 2019-04-05 14:54 UTC (permalink / raw)
To: Robin Murphy, linux-mm, Linux ARM, Jérôme Glisse,
Dan Williams, Will Deacon, Catalin Marinas
On 04/05/2019 07:07 PM, Robin Murphy wrote:
> On 05/04/2019 05:40, Anshuman Khandual wrote:
>> Hello,
>>
>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>> unmapping path through device_destroy(). Its device memory range end address
>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>> should retain it's values during the unmapping path as well. Is this assumption
>> right ?
>>
>> [ 62.779412] Call trace:
>> [ 62.779808] dump_backtrace+0x0/0x118
>> [ 62.780460] show_stack+0x14/0x20
>> [ 62.781204] dump_stack+0xa8/0xcc
>> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
>> [ 62.783021] devm_action_release+0x10/0x18
>> [ 62.783911] release_nodes+0x1b0/0x220
>> [ 62.784732] devres_release_all+0x34/0x50
>> [ 62.785623] device_release+0x24/0x90
>> [ 62.786454] kobject_put+0x74/0xe8
>> [ 62.787214] device_destroy+0x48/0x58
>> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
>> [ 62.789675] do_one_initcall+0x74/0x190
>> [ 62.790528] do_init_module+0x50/0x1c0
>> [ 62.791346] load_module+0x1be4/0x2140
>> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
>> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
>> [ 62.794128] el0_svc_handler+0x88/0x100
>> [ 62.794989] el0_svc+0x8/0xc
>>
>> The problem can be traced down here.
>>
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>> index e038e2b3b7ea..2a410c88c596 100644
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -33,7 +33,7 @@ struct devres {
>> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>> * buffer alignment as if it was allocated by plain kmalloc().
>> */
>> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>> + u8 __aligned(__alignof__(unsigned long long)) data[];
>> };
>>
>> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>>
>> dev_pagemap being added:
>>
>> #define ZONE_DEVICE_PHYS_START 0x680000000UL
>> #define ZONE_DEVICE_PHYS_END 0x6BFFFFFFFUL
>> #define ALTMAP_FREE 4096
>> #define ALTMAP_RESV 1024
>>
>> pgmap->type = MEMORY_DEVICE_PUBLIC;
>
> Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
struct dev_pagemap *pgmap;
pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
functions ? kzalloc() seems to solve the problem. Will do some more testing
tomorrow.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
2019-04-05 14:54 ` Anshuman Khandual
@ 2019-04-05 15:57 ` Robin Murphy
-1 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2019-04-05 15:57 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, Linux ARM, Jérôme Glisse,
Dan Williams, Will Deacon, Catalin Marinas
On 05/04/2019 15:54, Anshuman Khandual wrote:
>
>
> On 04/05/2019 07:07 PM, Robin Murphy wrote:
>> On 05/04/2019 05:40, Anshuman Khandual wrote:
>>> Hello,
>>>
>>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>>> unmapping path through device_destroy(). Its device memory range end address
>>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>>> should retain it's values during the unmapping path as well. Is this assumption
>>> right ?
>>>
>>> [ 62.779412] Call trace:
>>> [ 62.779808] dump_backtrace+0x0/0x118
>>> [ 62.780460] show_stack+0x14/0x20
>>> [ 62.781204] dump_stack+0xa8/0xcc
>>> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
>>> [ 62.783021] devm_action_release+0x10/0x18
>>> [ 62.783911] release_nodes+0x1b0/0x220
>>> [ 62.784732] devres_release_all+0x34/0x50
>>> [ 62.785623] device_release+0x24/0x90
>>> [ 62.786454] kobject_put+0x74/0xe8
>>> [ 62.787214] device_destroy+0x48/0x58
>>> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
>>> [ 62.789675] do_one_initcall+0x74/0x190
>>> [ 62.790528] do_init_module+0x50/0x1c0
>>> [ 62.791346] load_module+0x1be4/0x2140
>>> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
>>> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
>>> [ 62.794128] el0_svc_handler+0x88/0x100
>>> [ 62.794989] el0_svc+0x8/0xc
>>>
>>> The problem can be traced down here.
>>>
>>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>>> index e038e2b3b7ea..2a410c88c596 100644
>>> --- a/drivers/base/devres.c
>>> +++ b/drivers/base/devres.c
>>> @@ -33,7 +33,7 @@ struct devres {
>>> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>>> * buffer alignment as if it was allocated by plain kmalloc().
>>> */
>>> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>>> + u8 __aligned(__alignof__(unsigned long long)) data[];
>>> };
>>>
>>> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>>>
>>> dev_pagemap being added:
>>>
>>> #define ZONE_DEVICE_PHYS_START 0x680000000UL
>>> #define ZONE_DEVICE_PHYS_END 0x6BFFFFFFFUL
>>> #define ALTMAP_FREE 4096
>>> #define ALTMAP_RESV 1024
>>>
>>> pgmap->type = MEMORY_DEVICE_PUBLIC;
>>
>> Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
>
> struct dev_pagemap *pgmap;
>
> pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
>
> Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
> functions ? kzalloc() seems to solve the problem. Will do some more testing
> tomorrow.
The important point is that your pgmap is a devres allocation itself,
thus changing the alignment of devres::data is moving the thing which
was getting corrupted to a different relative location:
(gdb) p sizeof(struct devres) + sizeof(struct dev_pagemap)
$1 = 296
(gdb) p sizeof(struct devres) + (size_t)&(((struct dev_pagemap*)0)->res.end)
$2 = 192
vs.
(gdb) p sizeof(struct devres_node) + sizeof(struct dev_pagemap)
$3 = 192
(gdb) p sizeof(struct devres_node) + (size_t)&(((struct
dev_pagemap*)0)->res.end)
$4 = 88
The fact that the corruption of the 128-byte-aligned devres::data occurs
exactly where the 8-byte-aligned devres::data would have ended is
particularly suspicious...
Robin.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
@ 2019-04-05 15:57 ` Robin Murphy
0 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2019-04-05 15:57 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, Linux ARM, Jérôme Glisse,
Dan Williams, Will Deacon, Catalin Marinas
On 05/04/2019 15:54, Anshuman Khandual wrote:
>
>
> On 04/05/2019 07:07 PM, Robin Murphy wrote:
>> On 05/04/2019 05:40, Anshuman Khandual wrote:
>>> Hello,
>>>
>>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>>> unmapping path through device_destroy(). Its device memory range end address
>>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>>> should retain it's values during the unmapping path as well. Is this assumption
>>> right ?
>>>
>>> [ 62.779412] Call trace:
>>> [ 62.779808] dump_backtrace+0x0/0x118
>>> [ 62.780460] show_stack+0x14/0x20
>>> [ 62.781204] dump_stack+0xa8/0xcc
>>> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
>>> [ 62.783021] devm_action_release+0x10/0x18
>>> [ 62.783911] release_nodes+0x1b0/0x220
>>> [ 62.784732] devres_release_all+0x34/0x50
>>> [ 62.785623] device_release+0x24/0x90
>>> [ 62.786454] kobject_put+0x74/0xe8
>>> [ 62.787214] device_destroy+0x48/0x58
>>> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
>>> [ 62.789675] do_one_initcall+0x74/0x190
>>> [ 62.790528] do_init_module+0x50/0x1c0
>>> [ 62.791346] load_module+0x1be4/0x2140
>>> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
>>> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
>>> [ 62.794128] el0_svc_handler+0x88/0x100
>>> [ 62.794989] el0_svc+0x8/0xc
>>>
>>> The problem can be traced down here.
>>>
>>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>>> index e038e2b3b7ea..2a410c88c596 100644
>>> --- a/drivers/base/devres.c
>>> +++ b/drivers/base/devres.c
>>> @@ -33,7 +33,7 @@ struct devres {
>>> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>>> * buffer alignment as if it was allocated by plain kmalloc().
>>> */
>>> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>>> + u8 __aligned(__alignof__(unsigned long long)) data[];
>>> };
>>>
>>> On arm64 ARCH_KMALLOC_MINALIGN -> ARCH_DMA_MINALIGN -> 128
>>>
>>> dev_pagemap being added:
>>>
>>> #define ZONE_DEVICE_PHYS_START 0x680000000UL
>>> #define ZONE_DEVICE_PHYS_END 0x6BFFFFFFFUL
>>> #define ALTMAP_FREE 4096
>>> #define ALTMAP_RESV 1024
>>>
>>> pgmap->type = MEMORY_DEVICE_PUBLIC;
>>
>> Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
>
> struct dev_pagemap *pgmap;
>
> pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
>
> Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
> functions ? kzalloc() seems to solve the problem. Will do some more testing
> tomorrow.
The important point is that your pgmap is a devres allocation itself,
thus changing the alignment of devres::data is moving the thing which
was getting corrupted to a different relative location:
(gdb) p sizeof(struct devres) + sizeof(struct dev_pagemap)
$1 = 296
(gdb) p sizeof(struct devres) + (size_t)&(((struct dev_pagemap*)0)->res.end)
$2 = 192
vs.
(gdb) p sizeof(struct devres_node) + sizeof(struct dev_pagemap)
$3 = 192
(gdb) p sizeof(struct devres_node) + (size_t)&(((struct
dev_pagemap*)0)->res.end)
$4 = 88
The fact that the corruption of the 128-byte-aligned devres::data occurs
exactly where the 8-byte-aligned devres::data would have ended is
particularly suspicious...
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: struct dev_pagemap corruption
2019-04-05 14:54 ` Anshuman Khandual
@ 2019-04-07 22:25 ` Dan Williams
-1 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2019-04-07 22:25 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Robin Murphy, linux-mm, Linux ARM, Jérôme Glisse,
Will Deacon, Catalin Marinas
On Fri, Apr 5, 2019 at 7:54 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
[..]
> > Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
>
> struct dev_pagemap *pgmap;
>
> pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
>
> Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
> functions ?
On this specific question, no. devm_kzalloc() is how the pmem and
device-dax drivers allocate the pgmap passed to devm_memremap_pages().
The unwind order of the devres resources ensures that the
devm_memremap_pages() devres actions occur before the release of the
pgmap allocation.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: struct dev_pagemap corruption
@ 2019-04-07 22:25 ` Dan Williams
0 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2019-04-07 22:25 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Catalin Marinas, Will Deacon, linux-mm, Jérôme Glisse,
Robin Murphy, Linux ARM
On Fri, Apr 5, 2019 at 7:54 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
[..]
> > Given that what seems to ultimately get corrupted is the memory pointed to by pgmap here, how is *that* being allocated?
>
> struct dev_pagemap *pgmap;
>
> pgmap = devm_kzalloc(dev, sizeof(struct dev_pagemap), GFP_KERNEL);
>
> Is it problematic to use dev_kzalloc here instead of generic kmalloc/kzalloc
> functions ?
On this specific question, no. devm_kzalloc() is how the pmem and
device-dax drivers allocate the pgmap passed to devm_memremap_pages().
The unwind order of the devres resources ensures that the
devm_memremap_pages() devres actions occur before the release of the
pgmap allocation.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: struct dev_pagemap corruption
2019-04-05 4:40 ` Anshuman Khandual
@ 2019-04-05 13:42 ` Catalin Marinas
-1 siblings, 0 replies; 16+ messages in thread
From: Catalin Marinas @ 2019-04-05 13:42 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, Linux ARM, Jérôme Glisse, Dan Williams, Will Deacon
Hi Anshuman,
On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
[...]
> The problem can be traced down here.
>
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> * buffer alignment as if it was allocated by plain kmalloc().
> */
> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> + u8 __aligned(__alignof__(unsigned long long)) data[];
> };
[...]
> With the patch:
>
> [ 53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> [ 53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>
> Without the patch:
>
> [ 34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
> [ 34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
OK, so without this patch pgmap->res.end becomes 0 while it should stay
at 0x6bfffffff. Is it easy to reproduce with mainline?
What's zone_device_public_altmap_init? I couldn't grep it in mainline.
How's the pgmap allocated?
I'd suggest you enable kasan and see if it spots anything.
--
Catalin
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
@ 2019-04-05 13:42 ` Catalin Marinas
0 siblings, 0 replies; 16+ messages in thread
From: Catalin Marinas @ 2019-04-05 13:42 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, Jérôme Glisse, Will Deacon, Dan Williams, Linux ARM
Hi Anshuman,
On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
[...]
> The problem can be traced down here.
>
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> * buffer alignment as if it was allocated by plain kmalloc().
> */
> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> + u8 __aligned(__alignof__(unsigned long long)) data[];
> };
[...]
> With the patch:
>
> [ 53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
> [ 53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>
> Without the patch:
>
> [ 34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
> [ 34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
OK, so without this patch pgmap->res.end becomes 0 while it should stay
at 0x6bfffffff. Is it easy to reproduce with mainline?
What's zone_device_public_altmap_init? I couldn't grep it in mainline.
How's the pgmap allocated?
I'd suggest you enable kasan and see if it spots anything.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
2019-04-05 13:42 ` Catalin Marinas
@ 2019-04-05 14:58 ` Anshuman Khandual
-1 siblings, 0 replies; 16+ messages in thread
From: Anshuman Khandual @ 2019-04-05 14:58 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-mm, Linux ARM, Jérôme Glisse, Dan Williams, Will Deacon
On 04/05/2019 07:12 PM, Catalin Marinas wrote:
> Hi Anshuman,
>
> On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>> unmapping path through device_destroy(). Its device memory range end address
>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>> should retain it's values during the unmapping path as well. Is this assumption
>> right ?
> [...]
>> The problem can be traced down here.
>>
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>> index e038e2b3b7ea..2a410c88c596 100644
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -33,7 +33,7 @@ struct devres {
>> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>> * buffer alignment as if it was allocated by plain kmalloc().
>> */
>> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>> + u8 __aligned(__alignof__(unsigned long long)) data[];
>> };
> [...]
>> With the patch:
>>
>> [ 53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>> [ 53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>>
>> Without the patch:
>>
>> [ 34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
>> [ 34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
>
> OK, so without this patch pgmap->res.end becomes 0 while it should stay
> at 0x6bfffffff. Is it easy to reproduce with mainline?
Yeah but with some ZONE_DEVICE series patches.
>
> What's zone_device_public_altmap_init? I couldn't grep it in mainline.
Test module inti function (should have mentioned it clearly).
> How's the pgmap allocated?
devm_kzalloc() but the problem seems to be gone with kzalloc().
>
> I'd suggest you enable kasan and see if it spots anything.
>
Sure.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
@ 2019-04-05 14:58 ` Anshuman Khandual
0 siblings, 0 replies; 16+ messages in thread
From: Anshuman Khandual @ 2019-04-05 14:58 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-mm, Jérôme Glisse, Will Deacon, Dan Williams, Linux ARM
On 04/05/2019 07:12 PM, Catalin Marinas wrote:
> Hi Anshuman,
>
> On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
>> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
>> unmapping path through device_destroy(). Its device memory range end address
>> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
>> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
>> should retain it's values during the unmapping path as well. Is this assumption
>> right ?
> [...]
>> The problem can be traced down here.
>>
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>> index e038e2b3b7ea..2a410c88c596 100644
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -33,7 +33,7 @@ struct devres {
>> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
>> * buffer alignment as if it was allocated by plain kmalloc().
>> */
>> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
>> + u8 __aligned(__alignof__(unsigned long long)) data[];
>> };
> [...]
>> With the patch:
>>
>> [ 53.027865] XXX: zone_device_public_altmap_init pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>> [ 53.029840] XXX: devm_memremap_pages_release pgmap ffff8005de634218 resource ffff8005de634250 res->start 680000000 res->end 6bfffffff size 40000000
>>
>> Without the patch:
>>
>> [ 34.326066] XXX: zone_device_public_altmap_init pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 6bfffffff size 40000000
>> [ 34.328063] XXX: devm_memremap_pages_release pgmap ffff8005de530a80 resource ffff8005de530ab8 res->start 680000000 res->end 0 size fffffff980000001
>
> OK, so without this patch pgmap->res.end becomes 0 while it should stay
> at 0x6bfffffff. Is it easy to reproduce with mainline?
Yeah but with some ZONE_DEVICE series patches.
>
> What's zone_device_public_altmap_init? I couldn't grep it in mainline.
Test module inti function (should have mentioned it clearly).
> How's the pgmap allocated?
devm_kzalloc() but the problem seems to be gone with kzalloc().
>
> I'd suggest you enable kasan and see if it spots anything.
>
Sure.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: struct dev_pagemap corruption
2019-04-05 4:40 ` Anshuman Khandual
@ 2019-04-05 13:53 ` Jerome Glisse
-1 siblings, 0 replies; 16+ messages in thread
From: Jerome Glisse @ 2019-04-05 13:53 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, Linux ARM, Dan Williams, Will Deacon, Catalin Marinas
On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
> Hello,
>
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
>
> [ 62.779412] Call trace:
> [ 62.779808] dump_backtrace+0x0/0x118
> [ 62.780460] show_stack+0x14/0x20
> [ 62.781204] dump_stack+0xa8/0xcc
> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
> [ 62.783021] devm_action_release+0x10/0x18
> [ 62.783911] release_nodes+0x1b0/0x220
> [ 62.784732] devres_release_all+0x34/0x50
> [ 62.785623] device_release+0x24/0x90
> [ 62.786454] kobject_put+0x74/0xe8
> [ 62.787214] device_destroy+0x48/0x58
> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
> [ 62.789675] do_one_initcall+0x74/0x190
> [ 62.790528] do_init_module+0x50/0x1c0
> [ 62.791346] load_module+0x1be4/0x2140
> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
> [ 62.794128] el0_svc_handler+0x88/0x100
> [ 62.794989] el0_svc+0x8/0xc
>
> The problem can be traced down here.
>
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> * buffer alignment as if it was allocated by plain kmalloc().
> */
> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> + u8 __aligned(__alignof__(unsigned long long)) data[];
> };
I doubt that pgmap->res.end get corrupted during device_destroy() but given
that the above changes fix the issue it kind of boggle the mind. If i where
to debug this i would probably run a kernel with qemu -s to get a gdbserver
and then attach gdb and set breakpoint on devm_memremap_pages() then when
that trigger i would set memory watch on the pgmap->res.end (there use to be
way to use memory as pmem through kernel boot option).
A printk alternative solution is, assuming you only have one pgmap, add a
global static struct pgmap *debug_pgmap = NULL; in memremap.c set that in
devm_memremap_pages() and add an helper function:
void debug_pgmap(const char *file, unsigned line)
{
printk(... file, line);
printk(... debug_pmap->res.end);
}
In a header:
#define DEBUG_PGMAP debug_pgmap(__FILE__, __LINE__);
Then sprinkle DEBUG_PGMAP within device_destroy(), device_unregister(),
device_release() and see when it get corrupted.
gdb would be faster but sometime i got issue with memory watchpoint and virt.
Cheers,
Jérôme
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: struct dev_pagemap corruption
@ 2019-04-05 13:53 ` Jerome Glisse
0 siblings, 0 replies; 16+ messages in thread
From: Jerome Glisse @ 2019-04-05 13:53 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, Dan Williams, Will Deacon, Linux ARM, Catalin Marinas
On Fri, Apr 05, 2019 at 10:10:22AM +0530, Anshuman Khandual wrote:
> Hello,
>
> On arm64 platform "struct dev_pagemap" is getting corrupted during ZONE_DEVICE
> unmapping path through device_destroy(). Its device memory range end address
> (pgmap->res.end) which is getting corrupted in this particular case. AFAICS
> pgmap which gets initialized by the driver and mapped with devm_memremap_pages()
> should retain it's values during the unmapping path as well. Is this assumption
> right ?
>
> [ 62.779412] Call trace:
> [ 62.779808] dump_backtrace+0x0/0x118
> [ 62.780460] show_stack+0x14/0x20
> [ 62.781204] dump_stack+0xa8/0xcc
> [ 62.781941] devm_memremap_pages_release+0x24/0x1d8
> [ 62.783021] devm_action_release+0x10/0x18
> [ 62.783911] release_nodes+0x1b0/0x220
> [ 62.784732] devres_release_all+0x34/0x50
> [ 62.785623] device_release+0x24/0x90
> [ 62.786454] kobject_put+0x74/0xe8
> [ 62.787214] device_destroy+0x48/0x58
> [ 62.788041] zone_device_public_altmap_init+0x404/0x42c [zone_device_public_altmap]
> [ 62.789675] do_one_initcall+0x74/0x190
> [ 62.790528] do_init_module+0x50/0x1c0
> [ 62.791346] load_module+0x1be4/0x2140
> [ 62.792192] __se_sys_finit_module+0xb8/0xc8
> [ 62.793128] __arm64_sys_finit_module+0x18/0x20
> [ 62.794128] el0_svc_handler+0x88/0x100
> [ 62.794989] el0_svc+0x8/0xc
>
> The problem can be traced down here.
>
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index e038e2b3b7ea..2a410c88c596 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -33,7 +33,7 @@ struct devres {
> * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> * buffer alignment as if it was allocated by plain kmalloc().
> */
> - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[];
> + u8 __aligned(__alignof__(unsigned long long)) data[];
> };
I doubt that pgmap->res.end get corrupted during device_destroy() but given
that the above changes fix the issue it kind of boggle the mind. If i where
to debug this i would probably run a kernel with qemu -s to get a gdbserver
and then attach gdb and set breakpoint on devm_memremap_pages() then when
that trigger i would set memory watch on the pgmap->res.end (there use to be
way to use memory as pmem through kernel boot option).
A printk alternative solution is, assuming you only have one pgmap, add a
global static struct pgmap *debug_pgmap = NULL; in memremap.c set that in
devm_memremap_pages() and add an helper function:
void debug_pgmap(const char *file, unsigned line)
{
printk(... file, line);
printk(... debug_pmap->res.end);
}
In a header:
#define DEBUG_PGMAP debug_pgmap(__FILE__, __LINE__);
Then sprinkle DEBUG_PGMAP within device_destroy(), device_unregister(),
device_release() and see when it get corrupted.
gdb would be faster but sometime i got issue with memory watchpoint and virt.
Cheers,
Jérôme
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread