On Mon, Aug 28, 2023 at 02:32:59PM +0200, Christoph Hellwig wrote:

On Sun, Aug 27, 2023 at 10:45:18PM +0100, Al Viro wrote:

quoted

IOW, I suspect that the right thing to do would be something along the lines
of

The idea looks sensible to me, but we'll also need to do it for the
filemap_write_and_wait_range failure case.

Huh?  That's precisely where this patch is doing that...  That function
in mainline is
        if (unlikely(buffered_written < 0)) {
                if (direct_written)
                        return direct_written;
                return buffered_written;
        }

        /*
         * We need to ensure that the page cache pages are written to disk and
         * invalidated to preserve the expected O_DIRECT semantics.
         */
        err = filemap_write_and_wait_range(mapping, pos, end);
        if (err < 0) {
                /*
                 * We don't know how much we wrote, so just return the number of
                 * bytes which were direct-written
                 */
                if (direct_written)
                        return direct_written;
                return err;
        }
        invalidate_mapping_pages(mapping, pos >> PAGE_SHIFT, end >> PAGE_SHIFT);
        return direct_written + buffered_written;

The first failure exit does not need any work - the caller had not bumped
->ki_pos; the second one (after that 'if (err < 0) {' line) does and that's
where the patch upthread adds iocb->ki_pos -= buffered_written.

Or am I completely misparsing what you've written?