On Fri, May 02, 2025 at 09:18:40AM -0400, Brian Foster wrote:
> On Thu, May 01, 2025 at 03:22:29PM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 30, 2025 at 03:01:12PM -0400, Brian Foster wrote:
> > > iomap_write_begin() returns a folio based on current pos and
> > > remaining length in the iter, and each caller then trims the
> > > pos/length to the given folio. Clean this up a bit and let
> > > iomap_write_begin() return the trimmed range along with the folio.
> > >
> > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > > ---
> > > fs/iomap/buffered-io.c | 26 +++++++++++++++-----------
> > > 1 file changed, 15 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > > index d3b30ebad9ea..2fde268c39fc 100644
> > > --- a/fs/iomap/buffered-io.c
> > > +++ b/fs/iomap/buffered-io.c
> > > @@ -793,15 +793,22 @@ static int iomap_write_begin_inline(const struct iomap_iter *iter,
> > > return iomap_read_inline_data(iter, folio);
> > > }
> > >
> > > -static int iomap_write_begin(struct iomap_iter *iter, size_t len,
> > > - struct folio **foliop)
> > > +/*
> > > + * Grab and prepare a folio for write based on iter state. Returns the folio,
> > > + * offset, and length. Callers can optionally pass a max length *plen,
> > > + * otherwise init to zero.
> > > + */
> > > +static int iomap_write_begin(struct iomap_iter *iter, struct folio **foliop,
> > > + size_t *poffset, u64 *plen)
> >
> > Hmm, is this offset and length supposed to be bytes within the folio?
> > I find it a little odd that plen would be a u64 then, unless we're
> > preparing for folios that huge? Or is that just to avoid integer
> > truncation issues?
> >
>
> It was more the latter.. it's an input/output param for callers that use
> iomap_length(). That returns a u64, so just trying to keep things
> consistent. I suppose we could break the function decl into one param
> per line with "in/out" type comments if you think that is useful..?
willy tells me we actually /are/ trying to keep things clear for some
day supporting folio_size > 2G so I guess the u64/size_t usage here is
ok then. :)
--D
> Brian
>
> > --D
> >
> > > {
> > > const struct iomap_folio_ops *folio_ops = iter->iomap.folio_ops;
> > > const struct iomap *srcmap = iomap_iter_srcmap(iter);
> > > loff_t pos = iter->pos;
> > > + u64 len = min_t(u64, SIZE_MAX, iomap_length(iter));
> > > struct folio *folio;
> > > int status = 0;
> > >
> > > + len = *plen > 0 ? min_t(u64, len, *plen) : len;
> > > BUG_ON(pos + len > iter->iomap.offset + iter->iomap.length);
> > > if (srcmap != &iter->iomap)
> > > BUG_ON(pos + len > srcmap->offset + srcmap->length);
> > > @@ -833,8 +840,7 @@ static int iomap_write_begin(struct iomap_iter *iter, size_t len,
> > > }
> > > }
> > >
> > > - if (pos + len > folio_pos(folio) + folio_size(folio))
> > > - len = folio_pos(folio) + folio_size(folio) - pos;
> > > + pos = iomap_trim_folio_range(iter, folio, poffset, &len);
> > >
> > > if (srcmap->type == IOMAP_INLINE)
> > > status = iomap_write_begin_inline(iter, folio);
> > > @@ -847,6 +853,7 @@ static int iomap_write_begin(struct iomap_iter *iter, size_t len,
> > > goto out_unlock;
> > >
> > > *foliop = folio;
> > > + *plen = len;
> > > return 0;
> > >
> > > out_unlock:
> > > @@ -967,7 +974,7 @@ static int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
> > > break;
> > > }
> > >
> > > - status = iomap_write_begin(iter, bytes, &folio);
> > > + status = iomap_write_begin(iter, &folio, &offset, &bytes);
> > > if (unlikely(status)) {
> > > iomap_write_failed(iter->inode, iter->pos, bytes);
> > > break;
> > > @@ -975,7 +982,7 @@ static int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
> > > if (iter->iomap.flags & IOMAP_F_STALE)
> > > break;
> > >
> > > - pos = iomap_trim_folio_range(iter, folio, &offset, &bytes);
> > > + pos = iter->pos;
> > >
> > > if (mapping_writably_mapped(mapping))
> > > flush_dcache_folio(folio);
> > > @@ -1295,14 +1302,12 @@ static int iomap_unshare_iter(struct iomap_iter *iter)
> > > bool ret;
> > >
> > > bytes = min_t(u64, SIZE_MAX, bytes);
> > > - status = iomap_write_begin(iter, bytes, &folio);
> > > + status = iomap_write_begin(iter, &folio, &offset, &bytes);
> > > if (unlikely(status))
> > > return status;
> > > if (iomap->flags & IOMAP_F_STALE)
> > > break;
> > >
> > > - iomap_trim_folio_range(iter, folio, &offset, &bytes);
> > > -
> > > ret = iomap_write_end(iter, bytes, bytes, folio);
> > > __iomap_put_folio(iter, bytes, folio);
> > > if (WARN_ON_ONCE(!ret))
> > > @@ -1367,7 +1372,7 @@ static int iomap_zero_iter(struct iomap_iter *iter, bool *did_zero)
> > > bool ret;
> > >
> > > bytes = min_t(u64, SIZE_MAX, bytes);
> > > - status = iomap_write_begin(iter, bytes, &folio);
> > > + status = iomap_write_begin(iter, &folio, &offset, &bytes);
> > > if (status)
> > > return status;
> > > if (iter->iomap.flags & IOMAP_F_STALE)
> > > @@ -1376,7 +1381,6 @@ static int iomap_zero_iter(struct iomap_iter *iter, bool *did_zero)
> > > /* warn about zeroing folios beyond eof that won't write back */
> > > WARN_ON_ONCE(folio_pos(folio) > iter->inode->i_size);
> > >
> > > - iomap_trim_folio_range(iter, folio, &offset, &bytes);
> > > folio_zero_range(folio, offset, bytes);
> > > folio_mark_accessed(folio);
> > >
> > > --
> > > 2.49.0
> > >
> > >
> >
>
>
