[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] Fix failure cleanup in EvtchnFifoAcquire
Hi Paul, On 18/07/2025 09:49, Durrant, Paul wrote: >> -----Original Message----- >> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf >> Of Tu Dinh >> Sent: Friday, July 18, 2025 2:08 AM >> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx >> Cc: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>; Owen Smith <owen.smith@xxxxxxxxx> >> Subject: [EXTERNAL] [PATCH] Fix failure cleanup in EvtchnFifoAcquire >> >> CAUTION: This email originated from outside of the organization. Do not >> click links or open attachments unless you can confirm the sender and know >> the content is safe. >> >> >> >> The current index is incremented before checking for failure: >> > > I think that is by design. > >> while (Index < ProcessorCount) { >> Index++; >> [...] >> if (!NT_SUCCESS(status)) >> goto fail1; >> Context->ControlBlockMdl[vcpu_id] = Mdl; >> } >> >> Decrement the index before going into the cleanup loop to avoid calling >> __FreePage on invalid PMDLs. >> >> Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> >> --- >> src/xenbus/evtchn_fifo.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/xenbus/evtchn_fifo.c b/src/xenbus/evtchn_fifo.c >> index 1137dea..ed78815 100644 >> --- a/src/xenbus/evtchn_fifo.c >> +++ b/src/xenbus/evtchn_fifo.c >> @@ -561,6 +561,7 @@ fail1: >> >> EvtchnReset(); >> >> + Index--; > > I think this is wrong because... > >> while (--Index >= 0) { > > ... there is a pre-decrement here before the value of Index is used below. > > Paul It looks unintuitive, but Index is incremented very early into the main loop, before anything has a chance to fail. So Index will actually get double-incremented: Index = 0 while (Index < ProcessorCount) { vcpu_id = Index = 0; // as an example Index++; // 1 // success Context->ControlBlockMdl[vcpu_id] = Mdl; } while (Index < ProcessorCount) { vcpu_id = Index = 1; Index++; // 2 // failure goto fail; } fail: // the second loop incremented Index again without doing any work Index--; // 1 // Index is now the count of successful loop, so... while (--Index >= 0) { // need to decrement it again to get to the successful index // Index=0 } > >> unsigned int vcpu_id; >> >> -- >> 2.50.1.windows.1 >> >> >> >> Ngoc Tu Dinh | Vates XCP-ng Developer >> >> XCP-ng & Xen Orchestra - Vates solutions >> >> web: https://vates.tech >> > Ngoc Tu Dinh | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |