On 03.08.2023 11:30, Nicola Vetrini wrote: > On 03/08/2023 11:20, Jan Beulich wrote: >> On 02.08.2023 16:38, Nicola Vetrini wrote: >>> --- a/xen/arch/x86/mm/p2m-pod.c >>> +++ b/xen/arch/x86/mm/p2m-pod.c >>> @@ -1045,6 +1045,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const >>> gfn_t *gfns, unsigned int count >>> } >>> >>> return; >>> + ASSERT_UNREACHABLE(); >>> >>> out_unmap: >>> /* >> >> In the description you say "before", but here you add something _after_ >> "return". What's the deal? > > In this case the unreachable part is that after the label (looking at it > now, I should have > put the assert after the label to make it clear), because earlier all > jumps to > 'out_unmap' are like this: > > ASSERT_UNREACHABLE(); > domain_crash(d); > goto out_unmap; > > As I understood it, this is a defensive coding measure, preventing pages > to remain mapped if, > for some reason the above code actually executes. Am I correct?
The comment there says "probably". So the label and following code might be used for another error condition as well. Furthermore both paths presently using "goto out_unmap" already have ASSERT_UNREACHABLE(), so it's hard to see why we would need yet one more. Jan