On Sat, Jul 24, 2021 at 01:11:09PM +0800, Gao Xiang wrote: > On Sat, Jul 24, 2021 at 12:46:45PM +0800, Gao Xiang wrote: ... > > > > Thanks for the patch! > > > > Previously I'd like to skip the leading uptodate blocks and update the > > extent it covers that is due to already exist iop. If we get rid of the > > offset_in_page(pos) restriction like this, I'm not sure if we (or some > > other fs users) could face something like below (due to somewhat buggy > > fs users likewise): > > > > [0 - 4096) plain block > > > > [4096 - 4608) tail INLINE 1 (e.g. by mistake or just splitted > > .iomap_begin() report.) > > [4608 - 5120] tail INLINE 2 > > > > (cont.) > > So I think without the !offset_in_page(pos) restriction, at least we > may need to add something like: > > if (WARN_ON_ONCE(size != i_size_read(inode) - iomap->offset)) > return -EIO; > > to the approach to detect such cases at least but that is no need with > page-sized and !offset_in_page(pos) restriction. > Never mind, after rethinking with clear head, I think I was overthinking this part and it shouldn't behave like this. Sorry about the noise above. > > > > > > > > - addr = kmap_atomic(page); > > > + addr = kmap_atomic(page) + poff; > > > memcpy(addr, iomap_inline_buf(iomap, pos), size); > > > - memset(addr + size, 0, PAGE_SIZE - size); > > > + memset(addr + size, 0, PAGE_SIZE - poff - size); > > > kunmap_atomic(addr); > > > > As my limited understanding, this may need to be fixed, since it > > doesn't match kmap_atomic(page)... > > > > Thanks, > > Gao Xiang > > > > > - SetPageUptodate(page); > > > - return PAGE_SIZE; > > > + iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff); > > > + return PAGE_SIZE - poff; > > > } > > > > > > static inline bool iomap_block_needs_zeroing(struct inode *inode, > > > -- > > > 2.30.2 > > >
