Re: [PATCH 06/15] xfs: don't try to handle non-update pages in xfile_obj_load

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jan 03, 2024 at 08:41:17AM +0000, Christoph Hellwig wrote:
> shmem_read_mapping_page_gfp always returns an uptodate page or an
> ERR_PTR.  Remove the code that tries to handle a non-uptodate page.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Hmm.  xfile_pread calls shmem_read_mapping_page_gfp ->
shmem_read_folio_gfp -> shmem_get_folio_gfp(..., SGP_CACHE), right?

Therefore, if the page is !uptodate then the "clear:" code will mark it
uptodate, right?  And that's why xfile.c doesn't need to check uptodate?

If that's correct, then:
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D


> ---
>  fs/xfs/scrub/xfile.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> index 9e25ecf3dc2fec..46f4a06029cd4b 100644
> --- a/fs/xfs/scrub/xfile.c
> +++ b/fs/xfs/scrub/xfile.c
> @@ -166,18 +166,14 @@ xfile_obj_load(
>  			goto advance;
>  		}
>  
> -		if (PageUptodate(page)) {
> -			/*
> -			 * xfile pages must never be mapped into userspace, so
> -			 * we skip the dcache flush.
> -			 */
> -			kaddr = kmap_local_page(page);
> -			p = kaddr + offset_in_page(pos);
> -			memcpy(buf, p, len);
> -			kunmap_local(kaddr);
> -		} else {
> -			memset(buf, 0, len);
> -		}
> +		/*
> +		 * xfile pages must never be mapped into userspace, so
> +		 * we skip the dcache flush.
> +		 */
> +		kaddr = kmap_local_page(page);
> +		p = kaddr + offset_in_page(pos);
> +		memcpy(buf, p, len);
> +		kunmap_local(kaddr);
>  		put_page(page);
>  
>  advance:
> -- 
> 2.39.2
> 
>