Hi Vishal,
Sorry for the delay reply.
On 2022/12/6 4:34, Vishal Moola wrote:
On Tue, Nov 22, 2022 at 6:26 PM Vishal Moola <vishal.moola@xxxxxxxxx> wrote:
On Mon, Nov 14, 2022 at 1:38 PM Vishal Moola <vishal.moola@xxxxxxxxx> wrote:
On Sun, Nov 13, 2022 at 11:02 PM Chao Yu <chao@xxxxxxxxxx> wrote:
On 2022/10/18 4:24, Vishal Moola (Oracle) wrote:
Converted the function to use a folio_batch instead of pagevec. This is in
preparation for the removal of find_get_pages_range_tag().
Also modified f2fs_all_cluster_page_ready to take in a folio_batch instead
of pagevec. This does NOT support large folios. The function currently
Vishal,
It looks this patch tries to revert Fengnan's change:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=01fc4b9a6ed8eacb64e5609bab7ac963e1c7e486
How about doing some tests to evaluate its performance effect?
Yeah I'll play around with it to see how much of a difference it makes.
I did some testing. Looks like reverting Fengnan's change allows for
occasional, but significant, spikes in write latency. I'll work on a variation
of the patch that maintains the use of F2FS_ONSTACK_PAGES and send
that in the next version of the patch series. Thanks for pointing that out!
Following Matthew's comment, I'm thinking we should go with this patch
as is. The numbers between both variations did not have substantial
differences with regard to latency.
While the new variant would maintain the use of F2FS_ONSTACK_PAGES,
the code becomes messier and would end up limiting the number of
folios written back once large folio support is added. This means it would
have to be converted down to this version later anyways.
Does leaving this patch as is sound good to you?
For reference, here's what the version continuing to use a page
array of size F2FS_ONSTACK_PAGES would change:
+ nr_pages = 0;
+again:
+ nr_folios = filemap_get_folios_tag(mapping, &index, end,
+ tag, &fbatch);
+ if (nr_folios == 0) {
+ if (nr_pages)
+ goto write;
+ goto write;
Duplicated code.
break;
+ }
+ for (i = 0; i < nr_folios; i++) {
+ struct folio* folio = fbatch.folios[i];
+
+ idx = 0;
+ p = folio_nr_pages(folio);
+add_more:
+ pages[nr_pages] = folio_page(folio,idx);
+ folio_ref_inc(folio);
+ if (++nr_pages == F2FS_ONSTACK_PAGES) {
+ index = folio->index + idx + 1;
+ folio_batch_release(&fbatch);
+ goto write;
+ }
+ if (++idx < p)
+ goto add_more;
+ }
+ folio_batch_release(&fbatch);
+ goto again;
+write:
Looks fine to me, can you please send a formal patch?
Thanks,
How do the remaining f2fs patches in the series look to you?
Patch 16/23 f2fs_sync_meta_pages() in particular seems like it may
be prone to problems. If there are any changes that need to be made to
it I can include those in the next version as well.
Thanks for reviewing the patches so far. I wanted to follow up on asking
for review of the last couple of patches.
