On Wed, Mar 01, 2023 at 12:03:48AM +0530, Ritesh Harjani wrote: > Matthew Wilcox <willy@xxxxxxxxxxxxx> writes: > > > On Mon, Feb 27, 2023 at 01:13:30AM +0530, Ritesh Harjani (IBM) wrote: > >> +++ b/fs/iomap/buffered-io.c > >> @@ -535,11 +535,16 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos, > >> size_t from = offset_in_folio(folio, pos), to = from + len; > >> size_t poff, plen; > >> > >> + if (pos <= folio_pos(folio) && > >> + pos + len >= folio_pos(folio) + folio_size(folio)) > >> + return 0; > >> + > >> + iop = iomap_page_create(iter->inode, folio, iter->flags); > >> + > >> if (folio_test_uptodate(folio)) > >> return 0; > >> folio_clear_error(folio); > >> > >> - iop = iomap_page_create(iter->inode, folio, iter->flags); > >> if ((iter->flags & IOMAP_NOWAIT) && !iop && nr_blocks > 1) > >> return -EAGAIN; > > > > Don't you want to move the -EAGAIN check up too? Otherwise an > > io_uring write will dirty the entire folio rather than a block. > > I am not entirely convinced whether we should move this check up > (to put it just after the iop allocation). The reason is if the folio is > uptodate then it is ok to return 0 rather than -EAGAIN, because we are > anyway not going to read the folio from disk (given it is completely > uptodate). > > Thoughts? Or am I missing anything here. But then we won't have an iop, so a write will dirty the entire folio instead of just the blocks you want to dirty.
