[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen/blkfront: Only check REQ_FUA for writes
- To: Roger Pau Monne <roger.pau@xxxxxxxxxx>
- From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
- Date: Tue, 2 May 2023 16:40:17 +0000
- Accept-language: en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4mYyoRkb2KBysrxE5QbSTOtHlQEiOZdXXNJTxWMpNh8=; b=j8c1QcRoWnYQSVMzUxsedD/nGCzLkt+QT69PdIIyEKkkPijDpv7HV3bK9xrwOy7dlvBx3dOKavvAJbYNvyVRPuMjGQb6hXptWT9QwWH64H3rgQe1NYSnUBvozduVe/dXQi9S5eWGx7XyxPU+RSzBV2/55Ui7uNtagEraKQvLGW/hrQ4qVKC/XWw4KC6O2DN82j8K77AKS0+vxnbpdOsW1Ai/WZQ6Svqmi8qm3CTsxEVaE/wi1XH1g32f3GX6my4Y794VlAV3JpHIdiUXDRHve2P1Nvm4UmJEJUmvEes20GV7US7v0YobmO/Zuz9jx/p9Tqg5Y7sN6+qp0O6e59TmGQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Pt7AcETFJHnpYgkREil2HpNjnnM+VD6AqXHWcgo0Gb3zyh5CWX2qqx26fI6y6uJnSxdFXyzHxs2pzFuP+oQl6WIdsNm5gzGt9AFZADblCtUkzc5l3ldZqZcNFI7iYhCvvqXJcY5Yrsdmq15AWhIZVfCtkGPEm5pyinwzQXNK3QcFwRNdx9FwXcqQi7KG55CwV1+nSN6Hl4Wz9ZOP6BlJBHhNtH4qQHd9oKbZ7R2xI6y/RgYsThLjMTM8QKB2zvjkBrFnEetrY9aQHinX3QtQ1+U83Bo0QKbA2h4d+neirtVLsoG8dbUvYaEgxYlw46irhpM3/0NY5zwRdriIoJbY4Q==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "jgross@xxxxxxxx" <jgross@xxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "oleksandr_tyshchenko@xxxxxxxx" <oleksandr_tyshchenko@xxxxxxxx>, "axboe@xxxxxxxxx" <axboe@xxxxxxxxx>
- Delivery-date: Tue, 02 May 2023 16:40:43 +0000
- Ironport-data: A9a23:JW9YbKodScoReLkXi+lcRiD2+4NeBmL8ZBIvgKrLsJaIsI4StFCzt garIBnUPv3YYTH2ct91Pd+3/U4HuZTVmoNkTQNp/i80HnlA9JuZCYyVIHmrMnLJJKUvbq7FA +Y2MYCccZ9uHhcwgj/3b9ANeFEljfngqoLUUbKCYWYpA1c/Ek/NsDo788YhmIlknNOlNA2Ev NL2sqX3NUSsnjV5KQr40YrawP9UlKm06WJwUmAWP6gR5weDzyNNVvrzGInqR5fGatgMdgKFb 76rIIGRpgvx4xorA9W5pbf3GmVirmn6ZFXmZtJ+AsBOszAazsAA+v9T2Mk0MC+7vw6hjdFpo OihgLTrIesf0g8gr8xGO/VQO3kW0aSrY9YrK1Dn2SCY5xWun3cBX5yCpaz5VGEV0r8fPI1Ay RAXACIBTQuuucmY/IK+ScRhjZUqPOrZYqpK7xmMzRmBZRonabbqZvyQoPpnhnI3jM0IGuvCb c0EbzYpdA7HfxBEJlYQDtQ5gfusgX78NTZfrTp5p4JuuzSVkFM3juarbIO9lt+iHK25mm6Co XnduWDwDRwAK9WbzRKO8262h/+JliT+MG4XPOThrqY20ADJmgT/DjUGBEOk/KGooHKFYIoCd k092QcpoIEboRnDot7VGkfQTGS/lg4RXZ9cHvM37CmJy7HI+ECJC24cVDlDZdc68sgsSlQC1 FCTmMjyLSdyq7DTQnWYnp+Pti+7MyURKW4EZAcHQBED7t2lp5s85jrISttgC6ezgsfCBSDrw zuKoS49gJ0elccOka68+DjviiKmoZXhTQMv4AjTGG6mhj6Vf6agbo2srF3Et/BJKd/DSkHb5 CRd3c+D8OoJEJeB0jSXR/kAF62o4PDDNyDAhVloHN8q8DHFF2OfQL28KQpWfC9BWvvosxeyC KMPkWu9PKNuAUY=
- Ironport-hdrordr: A9a23:N70KvqCEqzrvKHXlHela55DYdb4zR+YMi2TDt3oddfWaSKylfq GV7ZImPHrP4gr5N0tOpTntAse9qDbnhPxICOoqTNCftWvdyQiVxehZhOOP/9SjIVyaygc078 xdmsNFebnN5DZB7PoT4GODYqkdKNvsytHXuQ8JpU0dPD2DaMtbnndE4h7wKDwOeOHfb6BJaa Z14KB81kKdUEVSVOuXLF8fUdPOotXa/aiWHSLvV3YcmXKzZSrD0s+BLySl
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Msip_labels:
- Thread-index: AQHZeF3Nw+6VlmAXM0WLxiEiRURXM69HLPoAgAAImgI=
- Thread-topic: [PATCH] xen/blkfront: Only check REQ_FUA for writes
> From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> Sent: Tuesday, May 2, 2023 4:57 PM
> To: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx <xen-devel@xxxxxxxxxxxxxxxxxxxx>;
> jgross@xxxxxxxx <jgross@xxxxxxxx>; sstabellini@xxxxxxxxxx
> <sstabellini@xxxxxxxxxx>; oleksandr_tyshchenko@xxxxxxxx
> <oleksandr_tyshchenko@xxxxxxxx>; axboe@xxxxxxxxx <axboe@xxxxxxxxx>
> Subject: Re: [PATCH] xen/blkfront: Only check REQ_FUA for writes
>
> On Wed, Apr 26, 2023 at 05:40:05PM +0100, Ross Lagerwall wrote:
> > The existing code silently converts read operations with the
> > REQ_FUA bit set into write-barrier operations. This results in data
> > loss as the backend scribbles zeroes over the data instead of returning
> > it.
> >
> > While the REQ_FUA bit doesn't make sense on a read operation, at least
> > one well-known out-of-tree kernel module does set it and since it
> > results in data loss, let's be safe here and only look at REQ_FUA for
> > writes.
>
> Do we know what's the intention of the out-of-tree kernel module with
> it's usage of FUA for reads?
It was just a plain bug that has now been fixed:
https://github.com/veeam/blksnap/commit/e3b3e7369642b59e01c647934789e5e20b380c62
I think this patch is still worthwile since reads becoming writes is
asking for data corruption.
>
> Should this maybe be translated to a pair of flush cache and read
> requests?
>
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> > ---
> > drivers/block/xen-blkfront.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> > index 23ed258b57f0..c1890c8a9f6e 100644
> > --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -780,7 +780,8 @@ static int blkif_queue_rw_req(struct request *req,
> > struct blkfront_ring_info *ri
> > ring_req->u.rw.handle = info->handle;
> > ring_req->operation = rq_data_dir(req) ?
> > BLKIF_OP_WRITE : BLKIF_OP_READ;
> > - if (req_op(req) == REQ_OP_FLUSH || req->cmd_flags & REQ_FUA) {
> > + if (req_op(req) == REQ_OP_FLUSH ||
> > + (req_op(req) == REQ_OP_WRITE && (req->cmd_flags &
> > REQ_FUA))) {
>
> Should we print some kind of warning maybe once that we have received
> a READ request with the FUA flag set, and the FUA flag will have no
> effect?
>
I thought of adding something like this but I couldn't find any other
block layer code doing a similar check (also it seems more appropriate
in the core block layer).
WARN_ONCE(req_op(req) != REQ_OP_WRITE && (req->cmd_flags & REQ_FUA));
I can add it if the maintainers want it.
Thanks,
Ross
|
