On Wed, Nov 20, 2019 at 9:26 AM Jeff Moyer <jmoyer@xxxxxxxxxx> wrote: > > Pankaj Gupta <pagupta@xxxxxxxxxx> writes: > > > Remove logic to create child bio in the async flush function which > > causes child bio to get executed after parent bio 'pmem_make_request' > > completes. This resulted in wrong ordering of REQ_PREFLUSH with the > > data write request. > > > > Instead we are performing flush from the parent bio to maintain the > > correct order. Also, returning from function 'pmem_make_request' if > > REQ_PREFLUSH returns an error. > > > > Reported-by: Jeff Moyer <jmoyer@xxxxxxxxxx> > > Signed-off-by: Pankaj Gupta <pagupta@xxxxxxxxxx> > > There's a slight change in behavior for the error path in the > virtio_pmem driver. Previously, all errors from virtio_pmem_flush were > converted to -EIO. Now, they are reported as-is. I think this is > actually an improvement. > > I'll also note that the current behavior can result in data corruption, > so this should be tagged for stable. I added that and was about to push this out, but what about the fact that now the guest will synchronously wait for flushing to occur. The goal of the child bio was to allow that to be an I/O wait with overlapping I/O, or at least not blocking the submission thread. Does the block layer synchronously wait for PREFLUSH requests? If not I think a synchronous wait is going to be a significant performance regression. Are there any numbers to accompany this change?