[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 1/3] xen/blkfront: read response from backend only once
On 02.08.21 22:26, Julien Grall wrote: > Hi, > > On 02/08/2021 15:06, Oleksandr Andrushchenko wrote: >> On 30.07.21 13:38, Juergen Gross wrote: >>> In order to avoid problems in case the backend is modifying a response >>> on the ring page while the frontend has already seen it, just read the >>> response into a local buffer in one go and then operate on that buffer >>> only. >>> >>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx> >>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> >>> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> >>> --- >>> drivers/block/xen-blkfront.c | 35 ++++++++++++++++++----------------- >>> 1 file changed, 18 insertions(+), 17 deletions(-) >>> >>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c >>> index d83fee21f6c5..15e840287734 100644 >>> --- a/drivers/block/xen-blkfront.c >>> +++ b/drivers/block/xen-blkfront.c >>> @@ -1496,7 +1496,7 @@ static bool blkif_completion(unsigned long *id, >>> static irqreturn_t blkif_interrupt(int irq, void *dev_id) >>> { >>> struct request *req; >>> - struct blkif_response *bret; >>> + struct blkif_response bret; >>> RING_IDX i, rp; >>> unsigned long flags; >>> struct blkfront_ring_info *rinfo = (struct blkfront_ring_info >>> *)dev_id; >>> @@ -1513,8 +1513,9 @@ static irqreturn_t blkif_interrupt(int irq, void >>> *dev_id) >>> for (i = rinfo->ring.rsp_cons; i != rp; i++) { >>> unsigned long id; >>> - bret = RING_GET_RESPONSE(&rinfo->ring, i); >>> - id = bret->id; >>> + RING_COPY_RESPONSE(&rinfo->ring, i, &bret); >> >> As per my understanding copying is still not an atomic operation as the >> request/response >> >> are multi-byte structures in general. IOW, what prevents the backend from >> modifying the ring while >> >> we are copying? > > Nothing and, I believe, you are never going to be able to ensure atomicity > with large structure (at least between entity that doesn't trust each other). > > However, what you can do is copying the response once, check that it is > consistent and then use it. If it is not consistent, then you can report an > error. > > This is better than what's currently in tree. IOW we may have multiple read > so the code is prone to TOCTOU. Agree, Thanks > > Cheers, >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |