| Message ID | 20210415050448.267306-18-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add full read-write support for subpage | expand |
On 4/15/21 1:04 AM, Qu Wenruo wrote: > Just like read page, for subpage support we only require sector size > alignment. > > So change the error message condition to only require sector alignment. > > This should not affect existing code, as for regular sectorsize == > PAGE_SIZE case, we are still requiring page alignment. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/extent_io.c | 29 ++++++++++++----------------- > 1 file changed, 12 insertions(+), 17 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 53ac22e3560f..94f8b3ffe6a7 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2779,25 +2779,20 @@ static void end_bio_extent_writepage(struct bio *bio) > struct page *page = bvec->bv_page; > struct inode *inode = page->mapping->host; > struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); > + const u32 sectorsize = fs_info->sectorsize; > > - /* We always issue full-page reads, but if some block > - * in a page fails to read, blk_update_request() will > - * advance bv_offset and adjust bv_len to compensate. > - * Print a warning for nonzero offsets, and an error > - * if they don't add up to a full page. */ > - if (bvec->bv_offset || bvec->bv_len != PAGE_SIZE) { > - if (bvec->bv_offset + bvec->bv_len != PAGE_SIZE) > - btrfs_err(fs_info, > - "partial page write in btrfs with offset %u and length %u", > - bvec->bv_offset, bvec->bv_len); > - else > - btrfs_info(fs_info, > - "incomplete page write in btrfs with offset %u and length %u", > - bvec->bv_offset, bvec->bv_len); > - } > + /* Btrfs read write should always be sector aligned. */ > + if (!IS_ALIGNED(bvec->bv_offset, sectorsize)) > + btrfs_err(fs_info, > + "partial page write in btrfs with offset %u and length %u", > + bvec->bv_offset, bvec->bv_len); > + else if (!IS_ALIGNED(bvec->bv_len, sectorsize)) > + btrfs_info(fs_info, > + "incomplete page write with offset %u and length %u", > + bvec->bv_offset, bvec->bv_len); > > - start = page_offset(page); > - end = start + bvec->bv_offset + bvec->bv_len - 1; > + start = page_offset(page) + bvec->bv_offset; > + end = start + bvec->bv_len - 1; Does this bit work out for you now? Because before start was just the page offset. Clearly the way it was before is a bug (I think?), because it gets used in btrfs_writepage_endio_finish_ordered() with the start+len, so you really do want start = page_offset(page) + bv_offset. But this is a behavior change that warrants a patch of its own as it's unrelated to the sectorsize change. (Yes I realize I'm asking for more patches in an already huge series, yes I'm insane.) Thanks, Josef
On 2021/4/16 下午11:13, Josef Bacik wrote: > On 4/15/21 1:04 AM, Qu Wenruo wrote: >> Just like read page, for subpage support we only require sector size >> alignment. >> >> So change the error message condition to only require sector alignment. >> >> This should not affect existing code, as for regular sectorsize == >> PAGE_SIZE case, we are still requiring page alignment. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/extent_io.c | 29 ++++++++++++----------------- >> 1 file changed, 12 insertions(+), 17 deletions(-) >> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index 53ac22e3560f..94f8b3ffe6a7 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -2779,25 +2779,20 @@ static void end_bio_extent_writepage(struct >> bio *bio) >> struct page *page = bvec->bv_page; >> struct inode *inode = page->mapping->host; >> struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); >> + const u32 sectorsize = fs_info->sectorsize; >> - /* We always issue full-page reads, but if some block >> - * in a page fails to read, blk_update_request() will >> - * advance bv_offset and adjust bv_len to compensate. >> - * Print a warning for nonzero offsets, and an error >> - * if they don't add up to a full page. */ >> - if (bvec->bv_offset || bvec->bv_len != PAGE_SIZE) { >> - if (bvec->bv_offset + bvec->bv_len != PAGE_SIZE) >> - btrfs_err(fs_info, >> - "partial page write in btrfs with offset %u and >> length %u", >> - bvec->bv_offset, bvec->bv_len); >> - else >> - btrfs_info(fs_info, >> - "incomplete page write in btrfs with offset %u and >> length %u", >> - bvec->bv_offset, bvec->bv_len); >> - } >> + /* Btrfs read write should always be sector aligned. */ >> + if (!IS_ALIGNED(bvec->bv_offset, sectorsize)) >> + btrfs_err(fs_info, >> + "partial page write in btrfs with offset %u and length %u", >> + bvec->bv_offset, bvec->bv_len); >> + else if (!IS_ALIGNED(bvec->bv_len, sectorsize)) >> + btrfs_info(fs_info, >> + "incomplete page write with offset %u and length %u", >> + bvec->bv_offset, bvec->bv_len); >> - start = page_offset(page); >> - end = start + bvec->bv_offset + bvec->bv_len - 1; >> + start = page_offset(page) + bvec->bv_offset; >> + end = start + bvec->bv_len - 1; > > Does this bit work out for you now? At least generic passes here for my arm board. > Because before start was just the > page offset. Clearly the way it was before is a bug (I think?), because > it gets used in btrfs_writepage_endio_finish_ordered() with the > start+len, so you really do want start = page_offset(page) + bv_offset. Not a bug before, as for sectorsize == PAGE_SIZE case, all bvec has bv_offset == 0 and bv_len == PAGE_SIZE. Thanks, Qu > But this is a behavior change that warrants a patch of its own as it's > unrelated to the sectorsize change. (Yes I realize I'm asking for more > patches in an already huge series, yes I'm insane.) Thanks, > > Josef
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 53ac22e3560f..94f8b3ffe6a7 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2779,25 +2779,20 @@ static void end_bio_extent_writepage(struct bio *bio) struct page *page = bvec->bv_page; struct inode *inode = page->mapping->host; struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); + const u32 sectorsize = fs_info->sectorsize; - /* We always issue full-page reads, but if some block - * in a page fails to read, blk_update_request() will - * advance bv_offset and adjust bv_len to compensate. - * Print a warning for nonzero offsets, and an error - * if they don't add up to a full page. */ - if (bvec->bv_offset || bvec->bv_len != PAGE_SIZE) { - if (bvec->bv_offset + bvec->bv_len != PAGE_SIZE) - btrfs_err(fs_info, - "partial page write in btrfs with offset %u and length %u", - bvec->bv_offset, bvec->bv_len); - else - btrfs_info(fs_info, - "incomplete page write in btrfs with offset %u and length %u", - bvec->bv_offset, bvec->bv_len); - } + /* Btrfs read write should always be sector aligned. */ + if (!IS_ALIGNED(bvec->bv_offset, sectorsize)) + btrfs_err(fs_info, + "partial page write in btrfs with offset %u and length %u", + bvec->bv_offset, bvec->bv_len); + else if (!IS_ALIGNED(bvec->bv_len, sectorsize)) + btrfs_info(fs_info, + "incomplete page write with offset %u and length %u", + bvec->bv_offset, bvec->bv_len); - start = page_offset(page); - end = start + bvec->bv_offset + bvec->bv_len - 1; + start = page_offset(page) + bvec->bv_offset; + end = start + bvec->bv_len - 1; if (first_bvec) { btrfs_record_physical_zoned(inode, start, bio);
Just like read page, for subpage support we only require sector size alignment. So change the error message condition to only require sector alignment. This should not affect existing code, as for regular sectorsize == PAGE_SIZE case, we are still requiring page alignment. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent_io.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)