| Message ID | 20240129143502.189370-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/20] mm: move mapping_set_update out of <linux/swap.h> | expand |
On Mon, 29 Jan 2024, Christoph Hellwig wrote: > Export shmem_get_folio as a slightly lower-level variant of > shmem_read_folio_gfp. This will be useful for XFS xfile use cases > that want to pass SGP_NOALLOC or get a locked page, which the thin > shmem_read_folio_gfp wrapper can't provide. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > --- > mm/shmem.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/mm/shmem.c b/mm/shmem.c > index ad533b2f0721a7..dae684cd3c99fb 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -2137,12 +2137,27 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index, > return error; > } > > +/** > + * shmem_get_folio - find and get a reference to a shmem folio. > + * @inode: inode to search > + * @index: the page index. > + * @foliop: pointer to the found folio if one was found > + * @sgp: SGP_* flags to control behavior > + * > + * Looks up the page cache entry at @inode & @index. > + * > + * If this function returns a folio, it is returned with an increased refcount. If this function returns a folio, it is returned locked, with an increased refcount. (Important to mention that it's locked, since that differs from shmem_read_folio_gfp().) Hugh > + * > + * Return: The found folio, %NULL if SGP_READ or SGP_NOALLOC was passed in @sgp > + * and no folio was found at @index, or an ERR_PTR() otherwise. > + */ > int shmem_get_folio(struct inode *inode, pgoff_t index, struct folio **foliop, > enum sgp_type sgp) > { > return shmem_get_folio_gfp(inode, index, foliop, sgp, > mapping_gfp_mask(inode->i_mapping), NULL, NULL); > } > +EXPORT_SYMBOL_GPL(shmem_get_folio); > > /* > * This is like autoremove_wake_function, but it removes the wait queue > -- > 2.39.2
On Mon, Jan 29, 2024 at 03:34:47PM +0100, Christoph Hellwig wrote: > +/** > + * shmem_get_folio - find and get a reference to a shmem folio. > + * @inode: inode to search > + * @index: the page index. > + * @foliop: pointer to the found folio if one was found > + * @sgp: SGP_* flags to control behavior > + * > + * Looks up the page cache entry at @inode & @index. > + * > + * If this function returns a folio, it is returned with an increased refcount. > + * > + * Return: The found folio, %NULL if SGP_READ or SGP_NOALLOC was passed in @sgp > + * and no folio was found at @index, or an ERR_PTR() otherwise. I know I gave an R-b on this earlier, but Hugh made me look again, and this comment clearly does not reflect what the function does. Presumably it returns an errno and sets foliop if it returns 0? Also, should this function be called shmem_lock_folio() to mirror filemap_lock_folio()? > + */ > int shmem_get_folio(struct inode *inode, pgoff_t index, struct folio **foliop, > enum sgp_type sgp) > { > return shmem_get_folio_gfp(inode, index, foliop, sgp, > mapping_gfp_mask(inode->i_mapping), NULL, NULL); > } > +EXPORT_SYMBOL_GPL(shmem_get_folio);
On Fri, Feb 16, 2024 at 01:53:09PM +0000, Matthew Wilcox wrote: > I know I gave an R-b on this earlier, but Hugh made me look again, and > this comment clearly does not reflect what the function does. > Presumably it returns an errno and sets foliop if it returns 0? Almost. With SGP_READ it can set *foliop to NULL and still return 0. > Also, should this function be called shmem_lock_folio() to mirror > filemap_lock_folio()? shmem_get_folio can also allocate (and sometimes zero) a new folio. Except for the different calling conventions that closest filemap equivalent is __filemap_get_folio. For now I'd like to avoid the bikeshedding on the name and just get the work done.
diff --git a/mm/shmem.c b/mm/shmem.c index ad533b2f0721a7..dae684cd3c99fb 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -2137,12 +2137,27 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index, return error; } +/** + * shmem_get_folio - find and get a reference to a shmem folio. + * @inode: inode to search + * @index: the page index. + * @foliop: pointer to the found folio if one was found + * @sgp: SGP_* flags to control behavior + * + * Looks up the page cache entry at @inode & @index. + * + * If this function returns a folio, it is returned with an increased refcount. + * + * Return: The found folio, %NULL if SGP_READ or SGP_NOALLOC was passed in @sgp + * and no folio was found at @index, or an ERR_PTR() otherwise. + */ int shmem_get_folio(struct inode *inode, pgoff_t index, struct folio **foliop, enum sgp_type sgp) { return shmem_get_folio_gfp(inode, index, foliop, sgp, mapping_gfp_mask(inode->i_mapping), NULL, NULL); } +EXPORT_SYMBOL_GPL(shmem_get_folio); /* * This is like autoremove_wake_function, but it removes the wait queue