On Mon 18-12-23 16:35:43, Christoph Hellwig wrote:
> From: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
>
> This simple helper will be the basis of the writeback iterator.
> To make this work, we need to remember the current index
> and end positions in writeback_control.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> [hch: heavily rebased, add helpers to get the tag and end index,
> don't keep the end index in struct writeback_control]
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Just two nits below. However you decide about them feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
> +static pgoff_t wbc_end(struct writeback_control *wbc)
> +{
> + if (wbc->range_cyclic)
> + return -1;
> + return wbc->range_end >> PAGE_SHIFT;
> +}
> +
> +static void writeback_get_batch(struct address_space *mapping,
> + struct writeback_control *wbc)
> +{
> + folio_batch_release(&wbc->fbatch);
> + cond_resched();
I'd prefer to have cond_resched() explicitely in the writeback loop instead
of hidden here in writeback_get_batch() where it logically does not make
too much sense to me...
> + filemap_get_folios_tag(mapping, &wbc->index, wbc_end(wbc),
> + wbc_to_tag(wbc), &wbc->fbatch);
> +}
> +
> /**
> * write_cache_pages - walk the list of dirty pages of the given address space and write all of them.
> * @mapping: address space structure to write
> @@ -2419,38 +2442,30 @@ int write_cache_pages(struct address_space *mapping,
> void *data)
> {
> int error;
> - int nr_folios;
> - pgoff_t index;
> pgoff_t end; /* Inclusive */
> - xa_mark_t tag;
>
> if (wbc->range_cyclic) {
> - index = mapping->writeback_index; /* prev offset */
> + wbc->index = mapping->writeback_index; /* prev offset */
> end = -1;
> } else {
> - index = wbc->range_start >> PAGE_SHIFT;
> + wbc->index = wbc->range_start >> PAGE_SHIFT;
> end = wbc->range_end >> PAGE_SHIFT;
> }
Maybe we should have:
end = wbc_end(wbc);
when we have the helper? But I guess this gets cleaned up in later patches
anyway so whatever.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
