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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 57A65CCA47F for ; Mon, 6 Jun 2022 19:26:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232266AbiFFT0E (ORCPT ); Mon, 6 Jun 2022 15:26:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49084 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232319AbiFFT0B (ORCPT ); Mon, 6 Jun 2022 15:26:01 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C332125C; Mon, 6 Jun 2022 12:26:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=4uHlez+Wc5PsSseu8aYUApVgEahEM4e0RFYu0c1sYbI=; b=Kguz+mv4nTNoE7IfIrHxfW12kN p5VHjYCZdFusZH8SYSRO4dIwuzX9+gwErsg8CgIn79S00CeIGXzh8lqEvqT4vjIhrKMc8FumEAPBQ uphr5TSNYnEv6iNvCfoZcq+H/ozILzx5sXkwNOtgEPGVvxE2VulgAy9HA/u0xG4waqpsDxy0msC/K L5Q2zWLwfR0d3YZo/1tYQKRGyhdpz4JFHyJrELB55HEaa6QGsflDDgbi7HlalDTzy3Y5tC/cO2jRP 482hmJOeiZBfwTBTJA0163FbHByeh1dmcvL0+4k7rKFWrJDSXznwm8f8Iel1ebU4uxlHOR+qQuHSl 0ngEwpOw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1nyIMN-00AwgO-NM; Mon, 06 Jun 2022 19:25:55 +0000 Date: Mon, 6 Jun 2022 20:25:55 +0100 From: Matthew Wilcox To: Stefan Roesch Cc: io-uring@vger.kernel.org, kernel-team@fb.com, linux-mm@kvack.org, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, david@fromorbit.com, jack@suse.cz, hch@infradead.org, axboe@kernel.dk Subject: Re: [PATCH v7 06/15] iomap: Return error code from iomap_write_iter() Message-ID: References: <20220601210141.3773402-1-shr@fb.com> <20220601210141.3773402-7-shr@fb.com> <0f83316c-3aa2-3cb6-ede1-c2dd2dd3ab31@fb.com> <4c15cc6f-97a6-ab53-6b1c-871058608419@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4c15cc6f-97a6-ab53-6b1c-871058608419@fb.com> Precedence: bulk List-ID: X-Mailing-List: io-uring@vger.kernel.org On Mon, Jun 06, 2022 at 12:21:28PM -0700, Stefan Roesch wrote: > > > On 6/6/22 12:18 PM, Matthew Wilcox wrote: > > On Mon, Jun 06, 2022 at 09:39:03AM -0700, Stefan Roesch wrote: > >> > >> > >> On 6/2/22 5:38 AM, Matthew Wilcox wrote: > >>> On Wed, Jun 01, 2022 at 02:01:32PM -0700, Stefan Roesch wrote: > >>>> Change the signature of iomap_write_iter() to return an error code. In > >>>> case we cannot allocate a page in iomap_write_begin(), we will not retry > >>>> the memory alloction in iomap_write_begin(). > >>> > >>> loff_t can already represent an error code. And it's already used like > >>> that. > >>> > >>>> @@ -829,7 +830,8 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) > >>>> length -= status; > >>>> } while (iov_iter_count(i) && length); > >>>> > >>>> - return written ? written : status; > >>>> + *processed = written ? written : error; > >>>> + return error; > >>> > >>> I think the change you really want is: > >>> > >>> if (status == -EAGAIN) > >>> return -EAGAIN; > >>> if (written) > >>> return written; > >>> return status; > >>> > >> > >> I think the change needs to be: > >> > >> - return written ? written : status; > >> + if (status == -EAGAIN) { > >> + iov_iter_revert(i, written); > >> + return -EAGAIN; > >> + } > >> + if (written) > >> + return written; > >> + return status; > > > > Ah yes, I think you're right. > > > > Does it work to leave everything the way it is, call back into the > > iomap_write_iter() after having done a short write, get the -EAGAIN at > > that point and pass the already-advanced iterator to the worker thread? > > I haven't looked into the details of how that works, so maybe you just > > can't do that. > > With the above change, short writes are handled correctly. Are they though? What about a write that crosses an extent boundary? iomap_write_iter() gets called once per extent and I have a feeling that you really need to revert the entire write, rather than just the part of the write that was to that extent. Anyway, my question was whether we need to revert or whether the worker thread can continue from where we left off.