| Message ID | 20221216150626.670312-5-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Turn iomap_page_ops into iomap_folio_ops | expand |
> +struct folio *iomap_folio_prepare(struct iomap_iter *iter, loff_t pos) > +{ > + unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS; > + > + if (iter->flags & IOMAP_NOWAIT) > + fgp |= FGP_NOWAIT; > + > + return __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT, > + fgp, mapping_gfp_mask(iter->inode->i_mapping)); > +} > +EXPORT_SYMBOL(iomap_folio_prepare); I'd name this __iomap_get_folio to match __filemap_get_folio. And all iomap exports are EXPORT_SYMBOL_GPL.
Am Fr., 23. Dez. 2022 um 16:22 Uhr schrieb Christoph Hellwig <hch@infradead.org>: > > +struct folio *iomap_folio_prepare(struct iomap_iter *iter, loff_t pos) > > +{ > > + unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS; > > + > > + if (iter->flags & IOMAP_NOWAIT) > > + fgp |= FGP_NOWAIT; > > + > > + return __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT, > > + fgp, mapping_gfp_mask(iter->inode->i_mapping)); > > +} > > +EXPORT_SYMBOL(iomap_folio_prepare); > > I'd name this __iomap_get_folio to match __filemap_get_folio. I was looking at it from the filesystem point of view: this helper is meant to be used in ->folio_prepare() handlers, so iomap_folio_prepare() seemed to be a better name than __iomap_get_folio(). > And all iomap exports are EXPORT_SYMBOL_GPL. Sure. Thanks, Andreas
On Fri, Dec 23, 2022 at 10:05:05PM +0100, Andreas Grünbacher wrote: > > I'd name this __iomap_get_folio to match __filemap_get_folio. > > I was looking at it from the filesystem point of view: this helper is > meant to be used in ->folio_prepare() handlers, so > iomap_folio_prepare() seemed to be a better name than > __iomap_get_folio(). Well, I think the right name for the methods that gets a folio is probably ->folio_get anyway.
On Fri, Dec 23, 2022 at 11:23:34PM -0800, Christoph Hellwig wrote: > On Fri, Dec 23, 2022 at 10:05:05PM +0100, Andreas Grünbacher wrote: > > > I'd name this __iomap_get_folio to match __filemap_get_folio. > > > > I was looking at it from the filesystem point of view: this helper is > > meant to be used in ->folio_prepare() handlers, so > > iomap_folio_prepare() seemed to be a better name than > > __iomap_get_folio(). > > Well, I think the right name for the methods that gets a folio is > probably ->folio_get anyway. For the a_ops, the convention I've been following is: folio_mark_dirty() -> aops->dirty_folio() -> iomap_dirty_folio() ie VERB_folio() as the name of the operation, and MODULE_VERB_folio() as the implementation. Seems to work pretty well.
On Sun, Dec 25, 2022 at 09:12:34AM +0000, Matthew Wilcox wrote: > > > I was looking at it from the filesystem point of view: this helper is > > > meant to be used in ->folio_prepare() handlers, so > > > iomap_folio_prepare() seemed to be a better name than > > > __iomap_get_folio(). > > > > Well, I think the right name for the methods that gets a folio is > > probably ->folio_get anyway. > > For the a_ops, the convention I've been following is: > > folio_mark_dirty() > -> aops->dirty_folio() > -> iomap_dirty_folio() > > ie VERB_folio() as the name of the operation, and MODULE_VERB_folio() > as the implementation. Seems to work pretty well. Yeay, ->get_folio sounds fine if not even better as it matches filemap_get_folio.
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 517ad5380a62..f0f167ca1b2e 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -457,6 +457,26 @@ bool iomap_is_partially_uptodate(struct folio *folio, size_t from, size_t count) } EXPORT_SYMBOL_GPL(iomap_is_partially_uptodate); +/** + * iomap_folio_prepare - get a folio reference for writing + * @iter: iteration structure + * @pos: start offset of write + * + * Returns a locked reference to the folio at @pos, or NULL if the folio could + * not be obtained. + */ +struct folio *iomap_folio_prepare(struct iomap_iter *iter, loff_t pos) +{ + unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS; + + if (iter->flags & IOMAP_NOWAIT) + fgp |= FGP_NOWAIT; + + return __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT, + fgp, mapping_gfp_mask(iter->inode->i_mapping)); +} +EXPORT_SYMBOL(iomap_folio_prepare); + bool iomap_release_folio(struct folio *folio, gfp_t gfp_flags) { trace_iomap_release_folio(folio->mapping->host, folio_pos(folio), @@ -603,12 +623,8 @@ static int iomap_write_begin(struct iomap_iter *iter, loff_t pos, const struct iomap_page_ops *page_ops = iter->iomap.page_ops; const struct iomap *srcmap = iomap_iter_srcmap(iter); struct folio *folio; - unsigned fgp = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE | FGP_NOFS; int status = 0; - if (iter->flags & IOMAP_NOWAIT) - fgp |= FGP_NOWAIT; - BUG_ON(pos + len > iter->iomap.offset + iter->iomap.length); if (srcmap != &iter->iomap) BUG_ON(pos + len > srcmap->offset + srcmap->length); @@ -625,8 +641,7 @@ static int iomap_write_begin(struct iomap_iter *iter, loff_t pos, return status; } - folio = __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT, - fgp, mapping_gfp_mask(iter->inode->i_mapping)); + folio = iomap_folio_prepare(iter, pos); if (!folio) { status = (iter->flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOMEM; iomap_folio_done(iter, pos, 0, NULL); diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 743e2a909162..0bf16ef22d81 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -261,6 +261,7 @@ int iomap_file_buffered_write_punch_delalloc(struct inode *inode, int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops); void iomap_readahead(struct readahead_control *, const struct iomap_ops *ops); bool iomap_is_partially_uptodate(struct folio *, size_t from, size_t count); +struct folio *iomap_folio_prepare(struct iomap_iter *iter, loff_t pos); bool iomap_release_folio(struct folio *folio, gfp_t gfp_flags); void iomap_invalidate_folio(struct folio *folio, size_t offset, size_t len); int iomap_file_unshare(struct inode *inode, loff_t pos, loff_t len,
Add an iomap_folio_prepare() helper that gets a folio reference based on an iomap iterator and an offset into the address space. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/iomap/buffered-io.c | 27 +++++++++++++++++++++------ include/linux/iomap.h | 1 + 2 files changed, 22 insertions(+), 6 deletions(-)