Re: [Cluster-devel] [RFC v3 4/7] iomap: Add iomap_folio_prepare helper
On Sun, Dec 25, 2022 at 09:12:34AM +, 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.
Re: [Cluster-devel] [RFC v3 4/7] iomap: Add iomap_folio_prepare helper
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.
Re: [Cluster-devel] [RFC v3 4/7] iomap: Add iomap_folio_prepare helper
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.
Re: [Cluster-devel] [RFC v3 4/7] iomap: Add iomap_folio_prepare helper
Am Fr., 23. Dez. 2022 um 16:22 Uhr schrieb Christoph Hellwig
:
> > +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
Re: [Cluster-devel] [RFC v3 4/7] iomap: Add iomap_folio_prepare helper
> +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.
[Cluster-devel] [RFC v3 4/7] iomap: Add iomap_folio_prepare helper
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
---
fs/iomap/buffered-io.c | 27 +--
include/linux/iomap.h | 1 +
2 files changed, 22 insertions(+), 6 deletions(-)
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 != >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,
--
2.38.1
