[ resend as the last attempt dropped all the cc's ] On Mon, May 21, 2018 at 4:10 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > On Mon, 21 May 2018 15:35:24 -0700 Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > >> The last step before devm_memremap_pages() returns success is to >> allocate a release action to tear the entire setup down. However, the >> result from devm_add_action() is not checked. >> >> Checking the error also means that we need to handle the fact that the >> percpu_ref may not be killed by the time devm_memremap_pages_release() >> runs. Add a new state flag for this case. >> >> Cc: <stable@xxxxxxxxxxxxxxx> >> Fixes: e8d513483300 ("memremap: change devm_memremap_pages interface...") >> Cc: Christoph Hellwig <hch@xxxxxx> >> Cc: "Jérôme Glisse" <jglisse@xxxxxxxxxx> >> Cc: Logan Gunthorpe <logang@xxxxxxxxxxxx> >> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> > > Why the cc:stable? The changelog doesn't describe the end-user-visible > impact of the bug (it always should, for this reason). True, I should have included that, one of these years I'll stop making this mistake. > > AFAICT we only go wrong when a small GFP_KERNEL allocation fails > (basically never happens), with undescribed results :( > Here's a better changelog: --- The last step before devm_memremap_pages() returns success is to allocate a release action to tear the entire setup down. However, the result from devm_add_action() is not checked. Checking the error also means that we need to handle the fact that the percpu_ref may not be killed by the time devm_memremap_pages_release() runs. Add a new state flag for this case. Without this change we could fail to register the teardown of devm_memremap_pages(). The likelihood of hitting this failure is tiny as small memory allocations almost always succeed. However, the impact of the failure is large given any future reconfiguration, or disable/enable, of an nvdimm namespace will fail forever as subsequent calls to devm_memremap_pages() will fail to setup the pgmap_radix since there will be stale entries for the physical address range.