Re: [PATCH] um: ubd: perform the flush the block layer asks for

[Johannes Berg <johannes@sipsolutions.net>]

On Sat, 2026-09-12 at 18:39 -0400, Mykyta Bozhenko wrote:
> ubd sets BLK_FEAT_WRITE_CACHE, so the block layer sends it REQ_OP_FLUSH
> requests and, as Documentation/block/writeback_cache_control.rst puts it,
> the driver "needs to handle them". do_io() does implement that: for
> REQ_OP_FLUSH it calls os_sync_file() on the backing file and maps the
> result back into the request.
> 
> That branch has been unreachable since commit fc6b6a872dcd ("um: ubd:
> Submit all data segments atomically"), which replaced the single
> per-request do_io() call with a loop over the request's data
> descriptors:
> 
> 	-			do_io((*io_req_buffer)[count]);
> 	+			for (i = 0; !req->error && i < req->desc_cnt; i++)
> 	+				do_io(req, &(req->io_desc[i]));
> 
> A flush carries no data and ubd_submit_request() sets desc_cnt to 0 for
> it, so the loop body never runs. The request is handed back to the block
> layer with error 0, i.e. the flush is reported as completed without the
> backing file ever being synced. Guest fsync(), fdatasync() and journal
> commits return success while the data is only in the host's page cache,
> and because no ordering is enforced either, a host crash can leave the
> image in a state the guest never allowed. The error path is dead too:
> a failing host fdatasync() cannot be reported.
> 
> Measured on a UML guest with ext4 on ubda, doing 20 writes of 4 KiB each
> followed by fdatasync(), then one fsync() and one directory fsync():
> 
>   before: the guest sees 22 successful flushes, /sys/block/ubda/stat
>           reports 16 completed flush requests, and the UML process issues
>           no fdatasync() on the image at all
>   after:  the same workload results in 43 fdatasync() calls on the image
> 
> os_pwrite_file() is entered 131 times either way, and e2fsck on the
> resulting image is clean in both cases.
> 
> Honouring the flush costs what the flush costs. With the image on host
> ext4, 400 iterations of write() plus fdatasync() in the guest take 467 ms
> before and 2007 ms after (medians of five runs). uretprobes on
> os_sync_file() attribute 1.597 s of that difference to time spent inside
> the host's fdatasync(), the remaining data path being unchanged
> (os_pwrite_file(): 19246 calls in both). A 64 MiB sequential write
> followed by a single fsync() goes from 278 ms to 362 ms, due to the
> periodic journal commits. With the image on tmpfs there is no measurable
> difference.
> 
> Users who prefer the previous speed to durability can disable the cache
> per device:
> 
>   echo "write through" > /sys/block/ubda/queue/write_cache
> 
> That is also cheaper than the unfixed driver, 326 ms for the same 400
> iterations, because the block layer then completes empty flush requests
> without entering the driver at all.

Yeah, you really need to not let LLMs write commit messages ...

johannes