Re: [RFC PATCH v1 15/18] mm: support write throttling for async buffered writes

[Jan Kara <jack@suse.cz>]

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.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR