On Sun, May 4, 2025 at 11:40 AM Bernd Schubert
<bernd.schubert@xxxxxxxxxxx> wrote:
>
>
>
> On 4/26/25 02:08, Joanne Koong wrote:
> > Add support for folios larger than one page size for writethrough
> > writes.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > fs/fuse/file.c | 15 ++++++++++-----
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index edc86485065e..e44b6d26c1c6 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -1146,7 +1146,8 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> > size_t tmp;
> > struct folio *folio;
> > pgoff_t index = pos >> PAGE_SHIFT;
> > - unsigned bytes = min(PAGE_SIZE - offset, num);
> > + unsigned int bytes;
> > + unsigned int folio_offset;
> >
> > again:
> > folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
> > @@ -1159,7 +1160,10 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> > if (mapping_writably_mapped(mapping))
> > flush_dcache_folio(folio);
> >
> > - tmp = copy_folio_from_iter_atomic(folio, offset, bytes, ii);
> > + folio_offset = ((index - folio->index) << PAGE_SHIFT) + offset;
> > + bytes = min(folio_size(folio) - folio_offset, num);
> > +
> > + tmp = copy_folio_from_iter_atomic(folio, folio_offset, bytes, ii);
> > flush_dcache_folio(folio);
> >
> > if (!tmp) {f
> > @@ -1180,6 +1184,7 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> >
> > err = 0;
> > ap->folios[ap->num_folios] = folio;
> > + ap->descs[ap->num_folios].offset = folio_offset;
> > ap->descs[ap->num_folios].length = tmp;
> > ap->num_folios++;
> >
> > @@ -1187,11 +1192,11 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> > pos += tmp;
> > num -= tmp;
> > offset += tmp;
> > - if (offset == PAGE_SIZE)
> > + if (offset == folio_size(folio))
> > offset = 0;
> >
> > - /* If we copied full page, mark it uptodate */
> > - if (tmp == PAGE_SIZE)
> > + /* If we copied full folio, mark it uptodate */
> > + if (tmp == folio_size(folio))
> > folio_mark_uptodate(folio);
>
> Here am I confused. I think tmp can be a subpart of the folio, let's say
> the folio is 2MB and somehow the again loop would iterate through the
> folio in smaller steps. So the folio would be entirely written out, but
> tmp might not be folio_size? Doesn't this need to sum up tmp for per
> folio and then use that value? And I actually wonder if we could use
> the above "(offset == folio_size(folio)" as well. At least if the
> initial offset for a folio is 0 it should work.
>
Hi Bernd,
Thanks for taking a look at this series and reviewing.
I don't think this scenario is possible. In copy_folio_iter_atomic()
which ends up calling into __copy_from_iter(), I don't see anywhere
where only the subpart of the folio can be copied out. The iter is a
ubuf, and from what I see, either copy_folio_iter_atomic() will return
0 or memcpy all requested bytes (unless bytes is greater than the
bytes contained in the iter, which isn't possible here) into the
folio.
Thanks,
Joanne
>
> Thanks,
> Bernd
>
> >
> > if (folio_test_uptodate(folio)) {
>
