On Fri, Jan 17, 2020 at 05:01:35PM +0200, Nikolay Borisov wrote:
> > @@ -3494,26 +3507,20 @@ static int write_dev_supers(struct btrfs_device *device,
> > BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> > crypto_shash_final(shash, sb->csum);
> >
> > - /* One reference for us, and we leave it for the caller */
> > - bh = __getblk(device->bdev, bytenr / BTRFS_BDEV_BLOCKSIZE,
> > - BTRFS_SUPER_INFO_SIZE);
> > - if (!bh) {
> > + page = find_or_create_page(device->bdev->bd_inode->i_mapping,
> > + bytenr >> PAGE_SHIFT, gfp_mask);
>
> You introduce asymmetry here. Because the write path now utilizes the
> page cache whereas the read path uses plain page alloc. I'm not sure but
> could this lead to reading garbage from the super block because now you
> don't have synchronization between the read and write path. This reminds
> me why I was using the page cache and not a plain page. Also by
> utilising the page cache you will potentially be reducing IO to disk
> since you can be fetching the sb data directly from cache.
There won't be any data in the cache anyway, because
btrfs_get_bdev_and_sb calls invalidate_bdev just before reading the
superblock. But otherwise I agree that the cache must be used,
effectively the same way as __bread does with BH.
