linux-kernel - Re: [PATCH] virtio pmem: fix async flush ordering

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPcyv4gCe8k1GdatAWn1991pm3QZq2WBFAGEFsZ2PXpyo2=wMw@mail.gmail.com>
Date:   Wed, 20 Nov 2019 23:23:46 -0800
From:   Dan Williams <dan.j.williams@...el.com>
To:     Jeff Moyer <jmoyer@...hat.com>
Cc:     Pankaj Gupta <pagupta@...hat.com>,
        linux-nvdimm <linux-nvdimm@...ts.01.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>,
        Vishal L Verma <vishal.l.verma@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        "Weiny, Ira" <ira.weiny@...el.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, Vivek Goyal <vgoyal@...hat.com>,
        Keith Busch <keith.busch@...el.com>
Subject: Re: [PATCH] virtio pmem: fix async flush ordering

On Wed, Nov 20, 2019 at 9:26 AM Jeff Moyer <jmoyer@...hat.com> wrote:
>
> Pankaj Gupta <pagupta@...hat.com> 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@...hat.com>
> > Signed-off-by: Pankaj Gupta <pagupta@...hat.com>
>
> 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?