On Tue, Nov 05, 2019 at 02:59:32PM +0100, Jan Kara wrote:
> On Tue 05-11-19 23:02:39, Matthew Bobrowski wrote:
> > + if (ret >= 0 && iov_iter_count(from)) {
> > + ssize_t err;
> > + loff_t endbyte;
> > +
> > + offset = iocb->ki_pos;
> > + err = ext4_buffered_write_iter(iocb, from);
> > + if (err < 0)
> > + return err;
> > +
> > + /*
> > + * We need to ensure that the pages within the page cache for
> > + * the range covered by this I/O are written to disk and
> > + * invalidated. This is in attempt to preserve the expected
> > + * direct I/O semantics in the case we fallback to buffered I/O
> > + * to complete off the I/O request.
> > + */
> > + ret += err;
> > + endbyte = offset + ret - 1;
> ^^ err here?
>
> Otherwise you would write out and invalidate too much AFAICT - the 'offset'
> is position just before we fall back to buffered IO. Otherwise this hunk
> looks good to me.
Er, yes. That's right, it should rather be 'err' instead or else we
would write/invalidate too much. I actually had this originally, but I
must've muddled it up while rewriting this patch on my other computer.
Thanks for picking that up!
/M
