Skip to contentSkip to conversation

kernel/memremap: Remove stale devres_free() call

PATCH In review Oliver O'Halloran 4 participants 4 messages Mar 6, 2018 libnvdimm: non-volatile memory device

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kernel/memremap: Remove stale devres_free() call

devm_memremap_pages() was re-worked in e8d513483300 to take a caller allocated struct dev_pagemap as a function parameter. A call to devres_free() was left in the error cleanup path which results in a kernel panic if the remap fails for some reason. Remove it to fix the panic and let devm_memremap_pages() fail gracefully.

Fixes e8d513483300 ("memremap: change devm_memremap_pages interface to use struct dev_pagemap")
Cc Logan Gunthorpe <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Dan Williams <[email protected]>

---
Both in-tree users of devm_memremap_pages() embed dev_pagemap into other structures so this shouldn't cause any leaks. Logan's p2p series does add one usage that assumes pgmap will be freed on error so that'll need fixing.

---
 kernel/memremap.c | 1 -
 1 file changed, 1 deletion(-)
kernel/memremap.c +0 -1
index 4dd4274cabe2..895e6b76b25e 100644
@@ -427,7 +427,6 @@ void *devm_memremap_pages(struct device *dev, struct dev_pagemap *pgmap)
  err_pfn_remap:
  err_radix:
 	pgmap_radix_release(res, pgoff);
-	devres_free(pgmap);
 	return ERR_PTR(error);
 }
 EXPORT_SYMBOL(devm_memremap_pages);
Signature · 6 lines
2.9.5

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Reviewed-by on v1 1/1
#2 lore raw
In reply to Oliver O'Halloran
Quoted 11 lines · Oliver O'Halloran

devm_memremap_pages() was re-worked in e8d513483300 to take a caller allocated struct dev_pagemap as a function parameter. A call to devres_free() was left in the error cleanup path which results in a kernel panic if the remap fails for some reason. Remove it to fix the panic and let devm_memremap_pages() fail gracefully.

Fixes e8d513483300 ("memremap: change devm_memremap_pages interface to use struct dev_pagemap")
Cc Logan Gunthorpe <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Dan Williams <[email protected]>
Quoted 5 lines

---
Both in-tree users of devm_memremap_pages() embed dev_pagemap into other structures so this shouldn't cause any leaks. Logan's p2p series does add one usage that assumes pgmap will be freed on error so that'll need fixing.

Yup, nice catch! I'll fix that for the next series.

Thanks,

Logan
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

#3 lore raw
Quoted 18 lines · Logan Gunthorpe
Quoted 7 lines · Oliver O'Halloran

devm_memremap_pages() was re-worked in e8d513483300 to take a caller allocated struct dev_pagemap as a function parameter. A call to devres_free() was left in the error cleanup path which results in a kernel panic if the remap fails for some reason. Remove it to fix the panic and let devm_memremap_pages() fail gracefully.

Quoted 6 lines
Fixes e8d513483300 ("memremap: change devm_memremap_pages interface to

use struct dev_pagemap")

Cc Logan Gunthorpe <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Dan Williams <[email protected]>
Quoted 1 line

Thanks, applied.
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm