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;
Date:   Fri, 20 Mar 2020 07:05:38 -0700
From:   Christoph Hellwig <hch@infradead.org>
To:     Goldwyn Rodrigues <rgoldwyn@suse.de>
Cc:     linux-fsdevel@vger.kernel.org, riteshh@linux.ibm.com,
        linux-ext4@vger.kernel.org, hch@infradead.org,
        darrick.wong@oracle.com, willy@infradead.org,
        linux-btrfs@vger.kernel.org, Josef Bacik <josef@toxicpanda.com>
Subject: Re: [PATCH v2] iomap: return partial I/O count on error in
 iomap_dio_bio_actor
Message-ID: <20200320140538.GA27895@infradead.org>
References: <20200319150805.uaggnfue5xgaougx@fiona>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200319150805.uaggnfue5xgaougx@fiona>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

I spent a fair amount of time looking over this change, and I am
starting to feel very bad about it.  iomap_apply() has pretty clear
semantics of either return an error, or return the bytes processed,
and in general these semantics work just fine.

The thing that breaks this concept is the btrfs submit_bio hook,
which allows the file system to keep state for each bio actually
submitted.  But I think you can simply keep the length internally
in btrfs - use the space in iomap->private as a counter of how
much was allocated, pass the iomap to the submit_io hook, and
update it there, and then deal with the rest in ->iomap_end.

That assumes ->iomap_end actually is the right place - can someone
explain what the expected call site for __endio_write_update_ordered
is?  It kinda sorta looks to me like something that would want to
be called after I/O completion, not after I/O submission, but maybe
I misunderstand the code.