[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [PATCH v2] Fix failure cleanup in EvtchnFifoAcquire
> -----Original Message----- > From: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> > Sent: Friday, July 18, 2025 11:54 AM > To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>; win-pv- > devel@xxxxxxxxxxxxxxxxxxxx > Cc: Owen Smith <owen.smith@xxxxxxxxx> > Subject: RE: [EXTERNAL] [PATCH v2] 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. > > > > On 18/07/2025 12:16, Durrant, Paul wrote: > >> -----Original Message----- > >> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On > Behalf > >> Of Tu Dinh > >> Sent: Friday, July 18, 2025 10:59 AM > >> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx > >> Cc: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>; Owen Smith > <owen.smith@xxxxxxxxx>; > >> Durrant, Paul <pdurrant@xxxxxxxxxxxx> > >> Subject: [EXTERNAL] [PATCH v2] 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: > >> > >> while (Index < ProcessorCount) { > >> Index++; > >> [...] > >> if (!NT_SUCCESS(status)) > >> goto fail1; > >> Context->ControlBlockMdl[vcpu_id] = Mdl; > >> } > >> > >> Make the main loop a normal for loop to avoid calling __FreePage on > >> invalid PMDLs. > >> > >> Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> > >> --- > >> src/xenbus/evtchn_fifo.c | 5 +---- > >> 1 file changed, 1 insertion(+), 4 deletions(-) > >> > >> diff --git a/src/xenbus/evtchn_fifo.c b/src/xenbus/evtchn_fifo.c > >> index 1137dea..fd06ef4 100644 > >> --- a/src/xenbus/evtchn_fifo.c > >> +++ b/src/xenbus/evtchn_fifo.c > >> @@ -502,16 +502,13 @@ EvtchnFifoAcquire( > >> > >> ProcessorCount = > >> KeQueryMaximumProcessorCountEx(ALL_PROCESSOR_GROUPS); > >> > >> - Index = 0; > >> - while (Index < ProcessorCount) { > >> + for (Index = 0; Index < ProcessorCount; Index++) { > > > > Prevailing style uses while loops I think so stick with that for > consistency, just move the increment down. > > I don't see how the increment could be moved down without breaking the > index skip when SystemProcessorVcpuId() == STATUS_NOT_SUPPORTED. > That's true, it would need a separate increment in that case. > I also find the while loop usage in EvtchnFifoAcquire very confusing, > and it was the cause of the cleanup bug in the first place. So I'd > prefer to make it more idiomatic this way. > More idiomatic but less consistent. Anyway... Reviewed-by: Paul Durrant <paul@xxxxxxx> > > > >> unsigned int vcpu_id; > >> PFN_NUMBER Pfn; > >> PHYSICAL_ADDRESS Address; > >> > >> status = SystemProcessorVcpuId(Index, &vcpu_id); > >> > >> - Index++; > >> - > >> if (status == STATUS_NOT_SUPPORTED) > >> continue; > >> > >> -- > >> 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 |