From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from verein.lst.de (verein.lst.de [213.95.11.211]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5E96B17E908; Wed, 12 Jun 2024 15:00:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.95.11.211 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718204440; cv=none; b=ru5Svhb/K7Hdtt5dqTrVf7tD816u9DEBbEfvXwAievlxn2AEOLn0QhxUo4Jk6KnaQc2nkoHdDzwpf1ChorHcv/E9m0qpM22R9M58Gz/dehIu/Zn/H8qEGr6RA1q+DcuH9Qn+8Uy3ggPhuy8fwMSuOAJlNLZVUH2lgq4Ps1gyvSo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718204440; c=relaxed/simple; bh=D9AKY16RIxKwWo5ffG5YBSyB4zSbRy1JLPHVsaztKec=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VZufRr/mp4lxVf+sOBILTJqZhJxQDNxR5XMmCsrAEhczCzxAPcIIl/fRYVkDp2Lvfh1keEmRaHFnb44k4BPbtrBYe5bObOkUwRIaiW6TRDUMUeLd2tXLr7CR11PwKC2gJwLa4tUiGrHwRxOpgzRaopjZi2qcO3STgpcUFCCegzA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=lst.de; spf=pass smtp.mailfrom=lst.de; arc=none smtp.client-ip=213.95.11.211 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=lst.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=lst.de Received: by verein.lst.de (Postfix, from userid 2407) id 9E11668AFE; Wed, 12 Jun 2024 17:00:30 +0200 (CEST) Date: Wed, 12 Jun 2024 17:00:30 +0200 From: Christoph Hellwig To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Cc: Christoph Hellwig , Jens Axboe , Geert Uytterhoeven , Richard Weinberger , Philipp Reisner , Lars Ellenberg , Christoph =?iso-8859-1?Q?B=F6hmwalder?= , Josef Bacik , Ming Lei , "Michael S. Tsirkin" , Jason Wang , Alasdair Kergon , Mike Snitzer , Mikulas Patocka , Song Liu , Yu Kuai , Vineeth Vijayan , "Martin K. Petersen" , linux-m68k@lists.linux-m68k.org, linux-um@lists.infradead.org, drbd-dev@lists.linbit.com, nbd@other.debian.org, linuxppc-dev@lists.ozlabs.org, ceph-devel@vger.kernel.org, virtualization@lists.linux.dev, xen-devel@lists.xenproject.org, linux-bcache@vger.kernel.org, dm-devel@lists.linux.dev, linux-raid@vger.kernel.org, linux-mmc@vger.kernel.org, linux-mtd@lists.infradead.org, nvdimm@lists.linux.dev, linux-nvme@lists.infradead.org, linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org, linux-block@vger.kernel.org Subject: Re: [PATCH 10/26] xen-blkfront: don't disable cache flushes when they fail Message-ID: <20240612150030.GA29188@lst.de> References: <20240611051929.513387-1-hch@lst.de> <20240611051929.513387-11-hch@lst.de> Precedence: bulk X-Mailing-List: ceph-devel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) On Wed, Jun 12, 2024 at 10:01:18AM +0200, Roger Pau Monné wrote: > On Tue, Jun 11, 2024 at 07:19:10AM +0200, Christoph Hellwig wrote: > > blkfront always had a robust negotiation protocol for detecting a write > > cache. Stop simply disabling cache flushes when they fail as that is > > a grave error. > > It's my understanding the current code attempts to cover up for the > lack of guarantees the feature itself provides: > So even when the feature is exposed, the backend might return > EOPNOTSUPP for the flush/barrier operations. How is this supposed to work? I mean in the worst case we could just immediately complete the flush requests in the driver, but we're really lying to any upper layer. > Such failure is tied on whether the underlying blkback storage > supports REQ_OP_WRITE with REQ_PREFLUSH operation. blkback will > expose "feature-barrier" and/or "feature-flush-cache" without knowing > whether the underlying backend supports those operations, hence the > weird fallback in blkfront. If we are just talking about the Linux blkback driver (I know there probably are a few other implementations) it won't every do that. I see it has code to do so, but the Linux block layer doesn't allow the flush operation to randomly fail if it was previously advertised. Note that even blkfront conforms to this as it fixes up the return value when it gets this notsupp error to ok. > Overall blkback should ensure that REQ_PREFLUSH is supported before > exposing "feature-barrier" or "feature-flush-cache", as then the > exposed features would really match what the underlying backend > supports (rather than the commands blkback knows about). Yes. The in-tree xen-blkback does that, but even without that the Linux block layer actually makes sure flushes sent by upper layers always succeed even when not supported. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 41D58C27C7B for ; Wed, 12 Jun 2024 15:00:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HuA4/TLZsUbfZYt5rHFuMf+GN8N+0h5D7wCpw49xtvc=; b=skaCHPBjl3dmRq 3VFJydQtEN7zrnbrCF8fXbPx7dNJYxrk4QW7RUXQQ0D+3dqN9D714UtId9DUs78X/XwFhxqJt6mjf Uil8n6FGfbXvbTU23jS/hLA1Mq5fIy8a5tbbjY8rbn8PRasTRYhG3l6TNCvx+yXNemv1t3OOZHbbi 0FCsWhmS025ro4/AOECeGEPnsU0KEOXy/LFVrid6XecubMQFs4E5CRK/7tDoXCYNm3BGPt5lMA5wC jyoPi1WIzAzB3+sPh66ieC/b4bif5dqDK7RqWuF7q7a701gehYqJwO2CPIgk+G1PejcGuMEPkzNwV yw1v2bATrqVsgLS2tnHQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sHPSs-0000000D3p5-0H53; Wed, 12 Jun 2024 15:00:42 +0000 Received: from verein.lst.de ([213.95.11.211]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sHPSm-0000000D3je-1bZS; Wed, 12 Jun 2024 15:00:37 +0000 Received: by verein.lst.de (Postfix, from userid 2407) id 9E11668AFE; Wed, 12 Jun 2024 17:00:30 +0200 (CEST) Date: Wed, 12 Jun 2024 17:00:30 +0200 From: Christoph Hellwig To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Cc: Christoph Hellwig , Jens Axboe , Geert Uytterhoeven , Richard Weinberger , Philipp Reisner , Lars Ellenberg , Christoph =?iso-8859-1?Q?B=F6hmwalder?= , Josef Bacik , Ming Lei , "Michael S. Tsirkin" , Jason Wang , Alasdair Kergon , Mike Snitzer , Mikulas Patocka , Song Liu , Yu Kuai , Vineeth Vijayan , "Martin K. Petersen" , linux-m68k@lists.linux-m68k.org, linux-um@lists.infradead.org, drbd-dev@lists.linbit.com, nbd@other.debian.org, linuxppc-dev@lists.ozlabs.org, ceph-devel@vger.kernel.org, virtualization@lists.linux.dev, xen-devel@lists.xenproject.org, linux-bcache@vger.kernel.org, dm-devel@lists.linux.dev, linux-raid@vger.kernel.org, linux-mmc@vger.kernel.org, linux-mtd@lists.infradead.org, nvdimm@lists.linux.dev, linux-nvme@lists.infradead.org, linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org, linux-block@vger.kernel.org Subject: Re: [PATCH 10/26] xen-blkfront: don't disable cache flushes when they fail Message-ID: <20240612150030.GA29188@lst.de> References: <20240611051929.513387-1-hch@lst.de> <20240611051929.513387-11-hch@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240612_080036_588385_C610925B X-CRM114-Status: GOOD ( 22.35 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org On Wed, Jun 12, 2024 at 10:01:18AM +0200, Roger Pau Monn=E9 wrote: > On Tue, Jun 11, 2024 at 07:19:10AM +0200, Christoph Hellwig wrote: > > blkfront always had a robust negotiation protocol for detecting a write > > cache. Stop simply disabling cache flushes when they fail as that is > > a grave error. > = > It's my understanding the current code attempts to cover up for the > lack of guarantees the feature itself provides: > So even when the feature is exposed, the backend might return > EOPNOTSUPP for the flush/barrier operations. How is this supposed to work? I mean in the worst case we could just immediately complete the flush requests in the driver, but we're really lying to any upper layer. > Such failure is tied on whether the underlying blkback storage > supports REQ_OP_WRITE with REQ_PREFLUSH operation. blkback will > expose "feature-barrier" and/or "feature-flush-cache" without knowing > whether the underlying backend supports those operations, hence the > weird fallback in blkfront. If we are just talking about the Linux blkback driver (I know there probably are a few other implementations) it won't every do that. I see it has code to do so, but the Linux block layer doesn't allow the flush operation to randomly fail if it was previously advertised. Note that even blkfront conforms to this as it fixes up the return value when it gets this notsupp error to ok. > Overall blkback should ensure that REQ_PREFLUSH is supported before > exposing "feature-barrier" or "feature-flush-cache", as then the > exposed features would really match what the underlying backend > supports (rather than the commands blkback knows about). Yes. The in-tree xen-blkback does that, but even without that the Linux block layer actually makes sure flushes sent by upper layers always succeed even when not supported. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E0C4EC27C77 for ; Wed, 12 Jun 2024 15:01:05 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4VzpdW54Yjz3frw for ; Thu, 13 Jun 2024 01:01:03 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=lst.de Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lst.de (client-ip=213.95.11.211; helo=verein.lst.de; envelope-from=hch@lst.de; receiver=lists.ozlabs.org) Received: from verein.lst.de (verein.lst.de [213.95.11.211]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4Vzpd35S3hz3cbB for ; Thu, 13 Jun 2024 01:00:38 +1000 (AEST) Received: by verein.lst.de (Postfix, from userid 2407) id 9E11668AFE; Wed, 12 Jun 2024 17:00:30 +0200 (CEST) Date: Wed, 12 Jun 2024 17:00:30 +0200 From: Christoph Hellwig To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Subject: Re: [PATCH 10/26] xen-blkfront: don't disable cache flushes when they fail Message-ID: <20240612150030.GA29188@lst.de> References: <20240611051929.513387-1-hch@lst.de> <20240611051929.513387-11-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nvdimm@lists.linux.dev, "Michael S. Tsirkin" , Jason Wang , linux-nvme@lists.infradead.org, Song Liu , linux-mtd@lists.infradead.org, Vineeth Vijayan , linux-bcache@vger.kernel.org, Christoph Hellwig , Alasdair Kergon , drbd-dev@lists.linbit.com, linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org, Richard Weinberger , Geert Uytterhoeven , Yu Kuai , dm-devel@lists.linux.dev, linux-um@lists.infradead.org, Mike Snitzer , Josef Bacik , Ming Lei , linux-raid@vger.kernel.org, linux-m68k@lists.linux-m68k.org, Mikulas Patocka , xen-devel@lists.xenproject.org, ceph-devel@vger.kernel.org, nbd@other.debian.org, Jens Axboe , linux-block@vger.kernel.org, "Martin K. Petersen" , linux-mmc@vger.kernel.org, Philipp Reisner , Christoph =?iso-8859-1?Q?B=F6hmwalder?= , virtualization@lists.linux.dev, Lars Ellenberg , linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Wed, Jun 12, 2024 at 10:01:18AM +0200, Roger Pau Monné wrote: > On Tue, Jun 11, 2024 at 07:19:10AM +0200, Christoph Hellwig wrote: > > blkfront always had a robust negotiation protocol for detecting a write > > cache. Stop simply disabling cache flushes when they fail as that is > > a grave error. > > It's my understanding the current code attempts to cover up for the > lack of guarantees the feature itself provides: > So even when the feature is exposed, the backend might return > EOPNOTSUPP for the flush/barrier operations. How is this supposed to work? I mean in the worst case we could just immediately complete the flush requests in the driver, but we're really lying to any upper layer. > Such failure is tied on whether the underlying blkback storage > supports REQ_OP_WRITE with REQ_PREFLUSH operation. blkback will > expose "feature-barrier" and/or "feature-flush-cache" without knowing > whether the underlying backend supports those operations, hence the > weird fallback in blkfront. If we are just talking about the Linux blkback driver (I know there probably are a few other implementations) it won't every do that. I see it has code to do so, but the Linux block layer doesn't allow the flush operation to randomly fail if it was previously advertised. Note that even blkfront conforms to this as it fixes up the return value when it gets this notsupp error to ok. > Overall blkback should ensure that REQ_PREFLUSH is supported before > exposing "feature-barrier" or "feature-flush-cache", as then the > exposed features would really match what the underlying backend > supports (rather than the commands blkback knows about). Yes. The in-tree xen-blkback does that, but even without that the Linux block layer actually makes sure flushes sent by upper layers always succeed even when not supported. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from verein.lst.de (verein.lst.de [213.95.11.211]) by mail19.linbit.com (LINBIT Mail Daemon) with ESMTP id 50B5542081C for ; Wed, 12 Jun 2024 17:00:34 +0200 (CEST) Date: Wed, 12 Jun 2024 17:00:30 +0200 From: Christoph Hellwig To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Subject: Re: [PATCH 10/26] xen-blkfront: don't disable cache flushes when they fail Message-ID: <20240612150030.GA29188@lst.de> References: <20240611051929.513387-1-hch@lst.de> <20240611051929.513387-11-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Cc: nvdimm@lists.linux.dev, "Michael S. Tsirkin" , Jason Wang , linux-nvme@lists.infradead.org, Song Liu , linux-mtd@lists.infradead.org, Vineeth Vijayan , linux-bcache@vger.kernel.org, Christoph Hellwig , Alasdair Kergon , drbd-dev@lists.linbit.com, linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org, Richard Weinberger , Geert Uytterhoeven , Yu Kuai , dm-devel@lists.linux.dev, linux-um@lists.infradead.org, Mike Snitzer , Josef Bacik , Ming Lei , linux-raid@vger.kernel.org, linux-m68k@lists.linux-m68k.org, Mikulas Patocka , xen-devel@lists.xenproject.org, ceph-devel@vger.kernel.org, nbd@other.debian.org, Jens Axboe , linux-block@vger.kernel.org, "Martin K. Petersen" , linux-mmc@vger.kernel.org, Philipp Reisner , virtualization@lists.linux.dev, Lars Ellenberg , linuxppc-dev@lists.ozlabs.org List-Id: "*Coordination* of development, patches, contributions -- *Questions* \(even to developers\) go to drbd-user, please." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Wed, Jun 12, 2024 at 10:01:18AM +0200, Roger Pau Monné wrote: > On Tue, Jun 11, 2024 at 07:19:10AM +0200, Christoph Hellwig wrote: > > blkfront always had a robust negotiation protocol for detecting a write > > cache. Stop simply disabling cache flushes when they fail as that is > > a grave error. > > It's my understanding the current code attempts to cover up for the > lack of guarantees the feature itself provides: > So even when the feature is exposed, the backend might return > EOPNOTSUPP for the flush/barrier operations. How is this supposed to work? I mean in the worst case we could just immediately complete the flush requests in the driver, but we're really lying to any upper layer. > Such failure is tied on whether the underlying blkback storage > supports REQ_OP_WRITE with REQ_PREFLUSH operation. blkback will > expose "feature-barrier" and/or "feature-flush-cache" without knowing > whether the underlying backend supports those operations, hence the > weird fallback in blkfront. If we are just talking about the Linux blkback driver (I know there probably are a few other implementations) it won't every do that. I see it has code to do so, but the Linux block layer doesn't allow the flush operation to randomly fail if it was previously advertised. Note that even blkfront conforms to this as it fixes up the return value when it gets this notsupp error to ok. > Overall blkback should ensure that REQ_PREFLUSH is supported before > exposing "feature-barrier" or "feature-flush-cache", as then the > exposed features would really match what the underlying backend > supports (rather than the commands blkback knows about). Yes. The in-tree xen-blkback does that, but even without that the Linux block layer actually makes sure flushes sent by upper layers always succeed even when not supported.