On 22/10/28 01:42PM, Matthew Wilcox wrote:
> On Fri, Oct 28, 2022 at 10:00:33AM +0530, Ritesh Harjani (IBM) wrote:
> > @@ -1354,7 +1399,8 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
> > * invalid, grab a new one.
> > */
> > for (i = 0; i < nblocks && pos < end_pos; i++, pos += len) {
> > - if (iop && !test_bit(i, iop->state))
> > + if (iop && (!test_bit(i, iop->state) ||
> > + !test_bit(i + nblocks, iop->state)))
> > continue;
> >
> > error = wpc->ops->map_blocks(wpc, inode, pos);
>
> Why do we need to test both uptodate and dirty? Surely we only need to
> test the dirty bit? How can a !uptodate block ever be marked as dirty?
Yes, you are right. We don't need to test uptodate bit.
In later revisions, I will correct that.
>
> More generally, I think open-coding this is going to lead to confusion.
> We need wrappers like 'iop_block_dirty()' and 'iop_block_uptodate()'.
Sure. Make sense. Thanks for the suggestion.
> (iop is still a bad name for this, but nobody's stepped up with a better
> one yet).
Looks fine to me :)
-ritesh
