On Thu, 27 Mar 2025, Eric Biggers wrote: > On Thu, Mar 27, 2025 at 06:12:22PM +0100, Mikulas Patocka wrote: > > > > > > On Thu, 27 Mar 2025, Eric Biggers wrote: > > > > > > Hi, Eric > > > > > > > > It seems that verity_prefetch_io doesn't work efficiently. > > > > dm_bufio_prefetch_with_ioprio > > > > __dm_bufio_prefetch > > > > blk_start_plug > > > > for loop > > > > __bufio_new > > > > submit_io > > > > cond_resched > > > > blk_finish_plug > > > > > > > > If more than one hash blocks need to be prefetched, cond_resched will > > > > be called in each loop and blk_finish_plug will be called at the end. > > > > > > > > The requests for hash blocks may have not been dispatched when the > > > > requests for data blocks have been completed. > > > > > > Well, it's always possible for the prefetch I/O to be too slow, but the way it > > > works does seem to be unnecessarily bad. The prefetch work runs on a kworker, > > > which is going to slow things down. The way it should work is that verity_map() > > > runs the prefetch work and starts, but doesn't wait for, any I/O that is needed. > > > (And of course, most of the time no I/O will actually be needed.) > > > > > > - Eric > > > > dm-verity used to prefetch from the verity_map function, but it caused a > > deadlock - 3b6b7813b198b578aa7e04e4047ddb8225c37b7f - so, it was moved to > > a workqueue. > > Interesting. Unfortunately I think the workqueue makes it way worse. > > In principle there is no need for it to block for anything. If it would have to > block, it just should just continue on. > > - Eric That seems to be right concern. Should I disable prefetch and add a switch to enable it on demand? Or delete the prefetch code entirely? There is also a bug that prefetch may race with suspend, sending I/Os even when the dm-verity device is suspended - but that is fixable by adding flush_workqueue to the postsuspend hook. Mikulas
