| Message ID | 20240103084126.513354-7-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/15] shmem: move the shmem_mapping assert into shmem_get_folio_gfp | expand |
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 > >
On Wed, Jan 03, 2024 at 03:55:38PM -0800, Darrick J. Wong wrote: > 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? Yes.
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:
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> --- fs/xfs/scrub/xfile.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)