Message ID | 20200416112608.8095-1-nborisov@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] btrfs: Make btrfs_read_disk_super return struct btrfs_disk_super | expand |
On 16/04/2020 13:26, Nikolay Borisov wrote: > Instead of returning both the page and the super block structure, make > btrfs_read_disk_super just return a pointer to struct btrfs_disk_super. > As a result the function signature is simplified. Uh I was under the impression I had done this when removing the buffer heads. Thanks for cleaning up behind me, Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Thu, Apr 16, 2020 at 02:26:08PM +0300, Nikolay Borisov wrote: > Instead of returning both the page and the super block structure, make > btrfs_read_disk_super just return a pointer to struct btrfs_disk_super. > As a result the function signature is simplified. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Added to misc-next, thanks. > @@ -1337,8 +1336,9 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags, > if (IS_ERR(bdev)) > return ERR_CAST(bdev); > > - if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) { > - device = ERR_PTR(-EINVAL); > + disk_super = btrfs_read_disk_super(bdev, bytenr); > + if (IS_ERR(disk_super)) { > + device = disk_super; With the ERR_CAST fixup. > goto error_bdev_put; > } > > -- > 2.17.1
Hi Nikolay, Thank you for the patch! Yet something to improve: [auto build test ERROR on v5.7-rc1] [also build test ERROR on next-20200416] [cannot apply to btrfs/next] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Make-btrfs_read_disk_super-return-struct-btrfs_disk_super/20200416-230011 base: 8f3d9f354286745c751374f5f1fcafee6b3f3136 config: x86_64-randconfig-s1-20200416 (attached as .config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): fs/btrfs/volumes.c: In function 'btrfs_scan_one_device': >> fs/btrfs/volumes.c:1341:10: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types] device = disk_super; ^ cc1: some warnings being treated as errors vim +1341 fs/btrfs/volumes.c 1309 1310 /* 1311 * Look for a btrfs signature on a device. This may be called out of the mount path 1312 * and we are not allowed to call set_blocksize during the scan. The superblock 1313 * is read via pagecache 1314 */ 1315 struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags, 1316 void *holder) 1317 { 1318 struct btrfs_super_block *disk_super; 1319 bool new_device_added = false; 1320 struct btrfs_device *device = NULL; 1321 struct block_device *bdev; 1322 u64 bytenr; 1323 1324 lockdep_assert_held(&uuid_mutex); 1325 1326 /* 1327 * we would like to check all the supers, but that would make 1328 * a btrfs mount succeed after a mkfs from a different FS. 1329 * So, we need to add a special mount option to scan for 1330 * later supers, using BTRFS_SUPER_MIRROR_MAX instead 1331 */ 1332 bytenr = btrfs_sb_offset(0); 1333 flags |= FMODE_EXCL; 1334 1335 bdev = blkdev_get_by_path(path, flags, holder); 1336 if (IS_ERR(bdev)) 1337 return ERR_CAST(bdev); 1338 1339 disk_super = btrfs_read_disk_super(bdev, bytenr); 1340 if (IS_ERR(disk_super)) { > 1341 device = disk_super; 1342 goto error_bdev_put; 1343 } 1344 1345 device = device_list_add(path, disk_super, &new_device_added); 1346 if (!IS_ERR(device)) { 1347 if (new_device_added) 1348 btrfs_free_stale_devices(path, device); 1349 } 1350 1351 btrfs_release_disk_super(disk_super); 1352 1353 error_bdev_put: 1354 blkdev_put(bdev, flags); 1355 1356 return device; 1357 } 1358 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Nikolay, Thank you for the patch! Yet something to improve: [auto build test ERROR on v5.7-rc1] [also build test ERROR on next-20200416] [cannot apply to btrfs/next] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Make-btrfs_read_disk_super-return-struct-btrfs_disk_super/20200416-230011 base: 8f3d9f354286745c751374f5f1fcafee6b3f3136 config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=sh If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): fs/btrfs/volumes.c: In function 'btrfs_scan_one_device': >> fs/btrfs/volumes.c:1341:10: error: assignment to 'struct btrfs_device *' from incompatible pointer type 'struct btrfs_super_block *' [-Werror=incompatible-pointer-types] 1341 | device = disk_super; | ^ cc1: some warnings being treated as errors vim +1341 fs/btrfs/volumes.c 1309 1310 /* 1311 * Look for a btrfs signature on a device. This may be called out of the mount path 1312 * and we are not allowed to call set_blocksize during the scan. The superblock 1313 * is read via pagecache 1314 */ 1315 struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags, 1316 void *holder) 1317 { 1318 struct btrfs_super_block *disk_super; 1319 bool new_device_added = false; 1320 struct btrfs_device *device = NULL; 1321 struct block_device *bdev; 1322 u64 bytenr; 1323 1324 lockdep_assert_held(&uuid_mutex); 1325 1326 /* 1327 * we would like to check all the supers, but that would make 1328 * a btrfs mount succeed after a mkfs from a different FS. 1329 * So, we need to add a special mount option to scan for 1330 * later supers, using BTRFS_SUPER_MIRROR_MAX instead 1331 */ 1332 bytenr = btrfs_sb_offset(0); 1333 flags |= FMODE_EXCL; 1334 1335 bdev = blkdev_get_by_path(path, flags, holder); 1336 if (IS_ERR(bdev)) 1337 return ERR_CAST(bdev); 1338 1339 disk_super = btrfs_read_disk_super(bdev, bytenr); 1340 if (IS_ERR(disk_super)) { > 1341 device = disk_super; 1342 goto error_bdev_put; 1343 } 1344 1345 device = device_list_add(path, disk_super, &new_device_added); 1346 if (!IS_ERR(device)) { 1347 if (new_device_added) 1348 btrfs_free_stale_devices(path, device); 1349 } 1350 1351 btrfs_release_disk_super(disk_super); 1352 1353 error_bdev_put: 1354 blkdev_put(bdev, flags); 1355 1356 return device; 1357 } 1358 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index c1909e5f4506..58a2c23ab91e 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1251,49 +1251,49 @@ void btrfs_release_disk_super(struct btrfs_super_block *super) put_page(page); } -static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr, - struct page **page, - struct btrfs_super_block **disk_super) +static struct btrfs_super_block *btrfs_read_disk_super(struct block_device *bdev, + u64 bytenr) { + struct btrfs_super_block *disk_super; + struct page *page; void *p; pgoff_t index; /* make sure our super fits in the device */ if (bytenr + PAGE_SIZE >= i_size_read(bdev->bd_inode)) - return 1; + return ERR_PTR(-EINVAL); /* make sure our super fits in the page */ - if (sizeof(**disk_super) > PAGE_SIZE) - return 1; + if (sizeof(*disk_super) > PAGE_SIZE) + return ERR_PTR(-EINVAL); /* make sure our super doesn't straddle pages on disk */ index = bytenr >> PAGE_SHIFT; - if ((bytenr + sizeof(**disk_super) - 1) >> PAGE_SHIFT != index) - return 1; + if ((bytenr + sizeof(*disk_super) - 1) >> PAGE_SHIFT != index) + return ERR_PTR(-EINVAL); /* pull in the page with our super */ - *page = read_cache_page_gfp(bdev->bd_inode->i_mapping, - index, GFP_KERNEL); + page = read_cache_page_gfp(bdev->bd_inode->i_mapping, index, + GFP_KERNEL); - if (IS_ERR(*page)) - return 1; + if (IS_ERR(page)) + return ERR_CAST(page); - p = page_address(*page); + p = page_address(page); /* align our pointer to the offset of the super block */ - *disk_super = p + offset_in_page(bytenr); + disk_super = p + offset_in_page(bytenr); - if (btrfs_super_bytenr(*disk_super) != bytenr || - btrfs_super_magic(*disk_super) != BTRFS_MAGIC) { + if (btrfs_super_bytenr(disk_super) != bytenr || + btrfs_super_magic(disk_super) != BTRFS_MAGIC) { btrfs_release_disk_super(p); - return 1; + return ERR_PTR(-EINVAL); } - if ((*disk_super)->label[0] && - (*disk_super)->label[BTRFS_LABEL_SIZE - 1]) - (*disk_super)->label[BTRFS_LABEL_SIZE - 1] = '\0'; + if (disk_super->label[0] && disk_super->label[BTRFS_LABEL_SIZE - 1]) + disk_super->label[BTRFS_LABEL_SIZE - 1] = '\0'; - return 0; + return disk_super; } int btrfs_forget_devices(const char *path) @@ -1319,7 +1319,6 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags, bool new_device_added = false; struct btrfs_device *device = NULL; struct block_device *bdev; - struct page *page; u64 bytenr; lockdep_assert_held(&uuid_mutex); @@ -1337,8 +1336,9 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags, if (IS_ERR(bdev)) return ERR_CAST(bdev); - if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) { - device = ERR_PTR(-EINVAL); + disk_super = btrfs_read_disk_super(bdev, bytenr); + if (IS_ERR(disk_super)) { + device = disk_super; goto error_bdev_put; }
Instead of returning both the page and the super block structure, make btrfs_read_disk_super just return a pointer to struct btrfs_disk_super. As a result the function signature is simplified. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- V2: * Make btrfs_scan_one_device forward the error it got from btrfs_read_disk_super to its callers. * Return EINVAL in case sb byte offset or magic value don't match in btrfs_read_disk_super * Removed explicit mention that read_cache_page_gfp can't return a NULL value. fs/btrfs/volumes.c | 48 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 24 deletions(-) -- 2.17.1