Matthew Wilcox <willy@xxxxxxxxxxxxx> writes: > 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. Ok, I got what you are saying. Make sense. I will give it a try. Thanks -ritesh
