| Message ID | 20230523081322.331337-4-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/16] btrfs: fix range_end calculation in extent_write_locked_range | expand |
On Tue, May 23, 2023 at 10:13:09AM +0200, Christoph Hellwig wrote: > Don't special case the fsverify error handling and clear the uptodate > bit for them as well like other readpage implementations (iomap, buffer, > mpage) do. > > Fixes: 146054090b08 ("btrfs: initial fsverity support") Wit Fixes: tag it means there's a bug but the the changelog does not read as abug description. Non-bug references to patches introducing some code can be done in the text. > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/btrfs/extent_io.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index fc48888742debd..4297478a7a625d 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -497,12 +497,8 @@ static void end_page_read(struct page *page, bool uptodate, u64 start, u32 len) > ASSERT(page_offset(page) <= start && > start + len <= page_offset(page) + PAGE_SIZE); > > - if (uptodate) { > - if (!btrfs_verify_page(page, start)) { > - btrfs_page_set_error(fs_info, page, start, len); > - } else { > - btrfs_page_set_uptodate(fs_info, page, start, len); > - } > + if (uptodate && btrfs_verify_page(page, start)) { > + btrfs_page_set_uptodate(fs_info, page, start, len); > } else { > btrfs_page_clear_uptodate(fs_info, page, start, len); > btrfs_page_set_error(fs_info, page, start, len); So the difference is that now !btrfs_verify_page() will forcibly clear uptodate. At this point it should not be set so comparing it to previous code it's a no-op. From the clarity POV the new code is better but I don't see any bug here.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index fc48888742debd..4297478a7a625d 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -497,12 +497,8 @@ static void end_page_read(struct page *page, bool uptodate, u64 start, u32 len) ASSERT(page_offset(page) <= start && start + len <= page_offset(page) + PAGE_SIZE); - if (uptodate) { - if (!btrfs_verify_page(page, start)) { - btrfs_page_set_error(fs_info, page, start, len); - } else { - btrfs_page_set_uptodate(fs_info, page, start, len); - } + if (uptodate && btrfs_verify_page(page, start)) { + btrfs_page_set_uptodate(fs_info, page, start, len); } else { btrfs_page_clear_uptodate(fs_info, page, start, len); btrfs_page_set_error(fs_info, page, start, len);
Don't special case the fsverify error handling and clear the uptodate bit for them as well like other readpage implementations (iomap, buffer, mpage) do. Fixes: 146054090b08 ("btrfs: initial fsverity support") Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/extent_io.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)