On 5/11/22 3:38 AM, Jan Kara wrote: > On Tue 10-05-22 13:16:30, Stefan Roesch wrote: >> On 5/10/22 2:50 AM, Jan Kara wrote: >>> I know that you're using fields in task_struct to propagate the delay info. >>> But IMHO that is unnecessary (although I don't care too much). Instead we >>> could factor out a variant of balance_dirty_pages() that returns 'pause' to >>> sleep, 0 if no sleeping needed. Normal balance_dirty_pages() would use this >>> for pause calculation, places wanting async throttling would only get the >>> pause to sleep. So e.g. iomap_write_iter() would then check and if returned >>> pause is > 0, it would abort the loop similary as we'd abort it for any >>> other reason when NOWAIT write is aborted because we need to sleep. Iouring >>> code then detects short write / EAGAIN and offloads the write to the >>> workqueue where normal balance_dirty_pages() can sleep as needed. >>> >>> This will make sure dirty limits are properly observed and we don't need >>> that much special handling for it. >>> >> >> I like the idea of factoring out a function out balance_dirty_pages(), however >> >> I see two challenges: >> - the write operation has already completed at this point, >> - so we can't really sleep on its completion in the io-worker in io-uring >> - we don't know how long to sleep in io-uring >> >> Currently balance_dirty_pages_ratelimited() is called at the end of the >> function iomap_write_iter(). If the function >> balance_dirty_pages_ratelimited() would instead be called at the >> beginning of the function iomap_write_iter() we could return -EAGAIN and >> then complete it in the io-worker. > > Well, we call balance_dirty_pages_ratelimited() after each page. So it does > not really matter much if the sleep is pushed to happen one page later. > balance_dirty_pages_ratelimited() does ratelimiting of when > balance_dirty_pages() are called so we have to make sure > current->nr_dirtied is not zeroed out before we really do wait (because > that is what determines whether we enter balance_dirty_pages() and how long > we sleep there) but looking at the code that should work out just fine. > I'll make the changes to balance_dirty_pages() for the next version of the patch series. > Honza
