Re: [RFCv2 3/9] arch/powerpc: Handle removing maybe-present bolted HPTEs
From: Paul Mackerras <hidden>
Date: 2016-02-08 05:47:19
On Fri, Jan 29, 2016 at 04:23:57PM +1100, David Gibson wrote:
At the moment the hpte_removebolted callback in ppc_md returns void and
will BUG_ON() if the hpte it's asked to remove doesn't exist in the first
place. This is awkward for the case of cleaning up a mapping which was
partially made before failing.
So, we add a return value to hpte_removebolted, and have it return ENOENT
in the case that the HPTE to remove didn't exist in the first place.
In the (sole) caller, we propagate errors in hpte_removebolted to its
caller to handle. However, we handle ENOENT specially, continuing to
complete the unmapping over the specified range before returning the error
to the caller.
This means that htab_remove_mapping() will work sanely on a partially
present mapping, removing any HPTEs which are present, while also returning
ENOENT to its caller in case it's important there.
There are two callers of htab_remove_mapping():
- In remove_section_mapping() we already WARN_ON() any error return,
which is reasonable - in this case the mapping should be fully
present
- In vmemmap_remove_mapping() we BUG_ON() any error. We change that to
just a WARN_ON() in the case of ENOENT, since failing to remove a
mapping that wasn't there in the first place probably shouldn't be
fatal.
Signed-off-by: David Gibson <redacted>[snip]
quoted hunk ↗ jump to hunk
--- a/arch/powerpc/mm/hash_utils_64.c +++ b/arch/powerpc/mm/hash_utils_64.c@@ -269,6 +269,7 @@ int htab_remove_mapping(unsigned long vstart, unsigned long vend, { unsigned long vaddr; unsigned int step, shift; + int rc = 0; shift = mmu_psize_defs[psize].shift; step = 1 << shift;@@ -276,10 +277,13 @@ int htab_remove_mapping(unsigned long vstart, unsigned long vend, if (!ppc_md.hpte_removebolted) return -ENODEV; - for (vaddr = vstart; vaddr < vend; vaddr += step) - ppc_md.hpte_removebolted(vaddr, psize, ssize); + for (vaddr = vstart; vaddr < vend; vaddr += step) { + rc = ppc_md.hpte_removebolted(vaddr, psize, ssize); + if ((rc < 0) && (rc != -ENOENT)) + return rc; + } - return 0; + return rc;
This will return the rc from the last hpte_removebolted call, which might be 0 even if earlier calls had returned -ENOENT. Or, if the last call fails with -ENOENT, this will return -ENOENT. Is that exactly what you meant? In the case where some calls to hpte_removebolted return -ENOENT, I would think we would want a consistent return value, which could be either 0 or -ENOENT, but it shouldn't depend on which specific calls fail with -ENOENT, in my opinion. Paul.