Received-SPF: pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
IronPort-SDR: uYx+vmlrfdHPtwJzHva0Uq64xOCU0KgUMb4WEDVZ3mD+r+tuxBACKvLxH65LK1br/9geoIBvOz
 tgWD/tuiEXrSD/wCeuuLX7JkbObBDt1wQNcsYmVbJcBxqd1dJHeFD/FBOt4PoswVgaAemj69M5
 jTtlKqYLMfeC7IxkIm8+JvIX93sZdyPbC4K55onveY+1hxToDIYLE1hXOJObv5/+ZJ2dEpTfGk
 U3eHjAuTO0justi0cb2Oc43xZnd/ijHXe2D9j7naGcIirB4rrfw8ENo/jpLUyVASkGQ1Nc4R0E
 9JQ=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Christoph Hellwig <hch@infradead.org>,
        Ritesh Harjani <riteshh@linux.ibm.com>
CC:     Goldwyn Rodrigues <rgoldwyn@suse.com>,
        "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "darrick.wong@oracle.com" <darrick.wong@oracle.com>
Subject: Re: [PATCH v2] iomap: return partial I/O count on error in
 iomap_dio_bio_actor
Thread-Topic: [PATCH v2] iomap: return partial I/O count on error in
 iomap_dio_bio_actor
Thread-Index: AQHV6AHTnTB6gZBg10G6wfTtrUQyIQ==
Date:   Wed, 26 Feb 2020 02:12:40 +0000
Message-ID: <BYAPR04MB58167849C121A55413A88012E7EA0@BYAPR04MB5816.namprd04.prod.outlook.com>
References: <20200220152355.5ticlkptc7kwrifz@fiona>
 <20200221045110.612705204E@d06av21.portsmouth.uk.ibm.com>
 <20200225205342.GA12066@infradead.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: f7b5b2a2-9f8e-4bd6-eb4b-08d7ba615b69
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Feb 2020 02:12:40.2547
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Ii1rbDL9mUgfyibnEYYuaTSY+5926VGeiX2WpoflDGfko8gRDfNqPfOM823cFoC1eW83pR4dZUkcKlCiF2usSg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB3830
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

On 2020/02/26 5:53, Christoph Hellwig wrote:=0A=
> On Fri, Feb 21, 2020 at 10:21:04AM +0530, Ritesh Harjani wrote:=0A=
>>>   		if (dio->error) {=0A=
>>>   			iov_iter_revert(dio->submit.iter, copied);=0A=
>>> -			copied =3D ret =3D 0;=0A=
>>> +			ret =3D 0;=0A=
>>>   			goto out;=0A=
>>>   		}=0A=
>>=0A=
>> But if I am seeing this correctly, even after there was a dio->error=0A=
>> if you return copied > 0, then the loop in iomap_dio_rw will continue=0A=
>> for next iteration as well. Until the second time it won't copy=0A=
>> anything since dio->error is set and from there I guess it may return=0A=
>> 0 which will break the loop.=0A=
> =0A=
> In addition to that copied is also iov_iter_reexpand call.  We don't=0A=
> really need the re-expand in case of errors, and in fact we also=0A=
> have the iov_iter_revert call before jumping out, so this will=0A=
> need a little bit more of an audit and properly documented in the=0A=
> commit log.=0A=
> =0A=
>>=0A=
>> Is this the correct flow? Shouldn't the while loop doing=0A=
>> iomap_apply in iomap_dio_rw should also break in case of=0A=
>> dio->error? Or did I miss anything?=0A=
> =0A=
> We'd need something there iff we care about a good number of written=0A=
> in case of the error.  Goldwyn, can you explain what you need this=0A=
> number for in btrfs?  Maybe with a pointer to the current code base?=0A=
=0A=
Not sure about btrfs, but for zonefs, getting the partial I/O count done fo=
r a=0A=
failed large dio would also be useful to avoid having to do the error recov=
ery=0A=
dance with report zones for getting the current zone write pointer.=0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=