Re: [PATCH] iomap: iomap_write_end cleanup

[Andreas Gruenbacher <agruenba@redhat.com>]

On Tue, May 3, 2022 at 11:53 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Tue, May 03, 2022 at 11:37:27PM +0200, Andreas Gruenbacher wrote:
> > In iomap_write_end(), only call iomap_write_failed() on the byte range
> > that has failed.  This should improve code readability, but doesn't fix
> > an actual bug because iomap_write_failed() is called after updating the
> > file size here and it only affects the memory beyond the end of the
> > file.
>
> I can't find a way to set 'ret' to anything other than 0 or len.  I know
> the code is written to make it look like we can return a short write,
> but I can't see a way to do it.

Good point, but that doesn't make the code any less confusing in my eyes.

Thanks,
Andreas

> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> > ---
> >  fs/iomap/buffered-io.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index 358ee1fb6f0d..8fb9b2797fc5 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -734,7 +734,7 @@ static size_t iomap_write_end(struct iomap_iter *iter, loff_t pos, size_t len,
> >       folio_put(folio);
> >
> >       if (ret < len)
> > -             iomap_write_failed(iter->inode, pos, len);
> > +             iomap_write_failed(iter->inode, pos + ret, len - ret);
> >       return ret;
> >  }
> >
> > --
> > 2.35.1
> >
>