Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 12 Dec 2022 15:48:13 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     Nhat Pham <nphamcs@gmail.com>
Cc:     akpm@linux-foundation.org, hannes@cmpxchg.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, bfoster@redhat.com,
        kernel-team@meta.com
Subject: Re: [PATCH v3 3/4] cachestat: implement cachestat syscall
Message-ID: <Y5dNPYVNyjHgGCD6@casper.infradead.org>
References: <20221208202808.908690-1-nphamcs@gmail.com>
 <20221208202808.908690-4-nphamcs@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221208202808.908690-4-nphamcs@gmail.com>
Precedence: bulk

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.

> +			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'.

Except that you may need to adjust nr if 'first_index' > folio->index, or
'last_index' < folio->index + nr.

You should test this with XFS, AFS or erofs to be sure you're getting
results that look right.