On Tue, Nov 27, 2018 at 08:57:58PM +0200, Nikolay Borisov wrote: > This function really checks whether adding more data to the bio will > straddle a stripe/chunk. So first let's give it a more appropraite > name - btrfs_bio_fits_in_stripe. Secondly, the offset parameter was > never used to just remove it. Thirdly, pages are submitted to either > btree or data inodes so it's guaranteed that tree->ops is set so replace > the check with an ASSERT. Finally, document the parameters of the > function. No functional changes. > > Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx> Reviewed-by: David Sterba <dsterba@xxxxxxxx> > - submit = btrfs_merge_bio_hook(page, 0, PAGE_SIZE, bio, 0); > + submit = btrfs_bio_fits_in_stripe(page, PAGE_SIZE, bio, > + 0); > - submit = btrfs_merge_bio_hook(page, 0, PAGE_SIZE, > - comp_bio, 0); > + submit = btrfs_bio_fits_in_stripe(page, PAGE_SIZE, > + comp_bio, 0); > - if (tree->ops && btrfs_merge_bio_hook(page, offset, page_size, > - bio, bio_flags)) > + ASSERT(tree->ops); > + if (btrfs_bio_fits_in_stripe(page, page_size, bio, bio_flags)) > can_merge = false; Got me curious if we could get rid of the size parameter, it's 2x PAGE_SIZE and could be something else in one case but it's not obvious if it really happens. Another thing I noticed is lack of proper error handling in all callers, as its' 0, 1, and negative errno. The error would be interpreted as true ie. add page to bio and continue.
