On Mon, Dec 12, 2022 at 7:48 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Thu, Dec 08, 2022 at 12:28:07PM -0800, Nhat Pham wrote:
> > + struct address_space *mapping = f.file->f_mapping;
> > + pgoff_t first_index = off >> PAGE_SHIFT;
> > + pgoff_t last_index =
> > + len == 0 ? ULONG_MAX : (off + len - 1) >> PAGE_SHIFT;
> > + XA_STATE(xas, &mapping->i_pages, first_index);
> > + struct folio *folio;
> > +
> > + rcu_read_lock();
> > + xas_for_each(&xas, folio, last_index) {
> > + if (xas_retry(&xas, folio) || !folio)
> > + continue;
>
> !folio can't be true. xas_for_each() terminates when folio is NULL.
Good to know - I'll remove this in the next version.
>
> > + if (xa_is_value(folio)) {
> > + /* page is evicted */
> > + void *shadow = (void *)folio;
> > + bool workingset; /* not used */
> > +
> > + cs.nr_evicted += 1;
> > +
> > +#ifdef CONFIG_SWAP /* implies CONFIG_MMU */
> > + if (shmem_mapping(mapping)) {
> > + /* shmem file - in swap cache */
> > + swp_entry_t swp = radix_to_swp_entry(folio);
> > +
> > + shadow = get_shadow_from_swap_cache(swp);
> > + }
> > +#endif
> > + if (workingset_test_recent(shadow, true, &workingset))
> > + cs.nr_recently_evicted += 1;
> > +
> > + continue;
> > + }
> > +
> > + /* page is in cache */
> > + cs.nr_cache += 1;
> > +
> > + if (folio_test_dirty(folio))
> > + cs.nr_dirty += 1;
> > +
> > + if (folio_test_writeback(folio))
> > + cs.nr_writeback += 1;
>
> A folio may represent more than one page. That's the point of folios.
> So there should be something in here which does
>
> unsigned long nr = folio_nr_pages();
>
> and then all these '1' should be 'nr'.
Yikes, that's a bug that I missed! Thanks for pointing it out.
> Except that you may need to adjust nr if 'first_index' > folio->index, or
> 'last_index' < folio->index + nr.
Sounds good!
>
> You should test this with XFS, AFS or erofs to be sure you're getting
> results that look right.
>
