diff mbox series

btrfs: scrub: fix an error in stripe offset calculation

Message ID c8f91363ab2e7ca24edbddf1feeca6c9fcf34f6e.1677033010.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: scrub: fix an error in stripe offset calculation | expand

Commit Message

Qu Wenruo Feb. 22, 2023, 2:30 a.m. UTC 
[BUG]
After commit "btrfs: replace btrfs_io_context::raid_map with a fixed u64
value", btrfs/261 fails:

 QA output created by 261
 ERROR: there are uncorrectable errors
 scrub failed to fix the fs for profile -m raid5 -d raid5
 ERROR: there are uncorrectable errors
 scrub failed to fix the fs for profile -m raid6 -d raid6
 Silence is golden

[CAUSE]
In commit "btrfs: replace btrfs_io_context::raid_map with a fixed u64
value", there is a call site using raid_map[i]:

		*stripe_offset = logical - raid_map[i];

That location is to calculate the offset inside the stripe.
But unfortunately the offending commit is using a wrong value:

		*stripe_offset = logical - full_stripe_logical;

The above line is change the behavior to "logical - raid_map[0]", not
"logical - raid_map[i]".

Thus causing wrong offset returned to the caller for raid56 replace.

[FIX]
Thankfully the call site itself doesn't really need to access
raid_map[i], since what we need is the offset inside the stripe.

So we can use BTRFS_STRIPE_LEN_MASK to calculate the offset inside the
stripe.

Please fold this one into the offending commit "btrfs: replace
btrfs_io_context::raid_map with a fixed u64 value".

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/scrub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

kernel test robot Feb. 22, 2023, 7:16 a.m. UTC | #1 
Hi Qu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20230221]
[cannot apply to kdave/for-next v6.2 v6.2-rc8 v6.2-rc7 linus/master v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/btrfs-scrub-fix-an-error-in-stripe-offset-calculation/20230222-103158
patch link:    https://lore.kernel.org/r/c8f91363ab2e7ca24edbddf1feeca6c9fcf34f6e.1677033010.git.wqu%40suse.com
patch subject: [PATCH] btrfs: scrub: fix an error in stripe offset calculation
config: i386-randconfig-a011 (https://download.01.org/0day-ci/archive/20230222/202302221532.TzU0U9gm-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ede724f4f7127f86931756949269770a7197339c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Qu-Wenruo/btrfs-scrub-fix-an-error-in-stripe-offset-calculation/20230222-103158
        git checkout ede724f4f7127f86931756949269770a7197339c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302221532.TzU0U9gm-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/btrfs/scrub.c:1456:6: error: use of undeclared identifier 'BTRFS_STRIPE_LEN_MASK'
                                    BTRFS_STRIPE_LEN_MASK;
                                    ^
   1 error generated.


vim +/BTRFS_STRIPE_LEN_MASK +1456 fs/btrfs/scrub.c

  1431	
  1432	static inline void scrub_stripe_index_and_offset(u64 logical, u64 map_type,
  1433							 u64 full_stripe_logical,
  1434							 int nstripes, int mirror,
  1435							 int *stripe_index,
  1436							 u64 *stripe_offset)
  1437	{
  1438		int i;
  1439	
  1440		if (map_type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
  1441			const int nr_data_stripes = (map_type & BTRFS_BLOCK_GROUP_RAID5) ?
  1442						    nstripes - 1 : nstripes - 2;
  1443	
  1444			/* RAID5/6 */
  1445			for (i = 0; i < nr_data_stripes; i++) {
  1446				const u64 data_stripe_start = full_stripe_logical +
  1447							(i * BTRFS_STRIPE_LEN);
  1448	
  1449				if (logical >= data_stripe_start &&
  1450				    logical < data_stripe_start + BTRFS_STRIPE_LEN)
  1451					break;
  1452			}
  1453	
  1454			*stripe_index = i;
  1455			*stripe_offset = (logical - full_stripe_logical) &
> 1456					 BTRFS_STRIPE_LEN_MASK;
  1457		} else {
  1458			/* The other RAID type */
  1459			*stripe_index = mirror;
  1460			*stripe_offset = 0;
  1461		}
  1462	}
  1463
kernel test robot Feb. 22, 2023, 8:58 a.m. UTC | #2 
Hi Qu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20230221]
[cannot apply to kdave/for-next v6.2 v6.2-rc8 v6.2-rc7 linus/master v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/btrfs-scrub-fix-an-error-in-stripe-offset-calculation/20230222-103158
patch link:    https://lore.kernel.org/r/c8f91363ab2e7ca24edbddf1feeca6c9fcf34f6e.1677033010.git.wqu%40suse.com
patch subject: [PATCH] btrfs: scrub: fix an error in stripe offset calculation
config: x86_64-randconfig-a011 (https://download.01.org/0day-ci/archive/20230222/202302221638.EMe7IGPV-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/ede724f4f7127f86931756949269770a7197339c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Qu-Wenruo/btrfs-scrub-fix-an-error-in-stripe-offset-calculation/20230222-103158
        git checkout ede724f4f7127f86931756949269770a7197339c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302221638.EMe7IGPV-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/btrfs/scrub.c: In function 'scrub_stripe_index_and_offset':
>> fs/btrfs/scrub.c:1456:34: error: 'BTRFS_STRIPE_LEN_MASK' undeclared (first use in this function); did you mean 'BTRFS_STRIPE_LEN'?
    1456 |                                  BTRFS_STRIPE_LEN_MASK;
         |                                  ^~~~~~~~~~~~~~~~~~~~~
         |                                  BTRFS_STRIPE_LEN
   fs/btrfs/scrub.c:1456:34: note: each undeclared identifier is reported only once for each function it appears in


vim +1456 fs/btrfs/scrub.c

  1431	
  1432	static inline void scrub_stripe_index_and_offset(u64 logical, u64 map_type,
  1433							 u64 full_stripe_logical,
  1434							 int nstripes, int mirror,
  1435							 int *stripe_index,
  1436							 u64 *stripe_offset)
  1437	{
  1438		int i;
  1439	
  1440		if (map_type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
  1441			const int nr_data_stripes = (map_type & BTRFS_BLOCK_GROUP_RAID5) ?
  1442						    nstripes - 1 : nstripes - 2;
  1443	
  1444			/* RAID5/6 */
  1445			for (i = 0; i < nr_data_stripes; i++) {
  1446				const u64 data_stripe_start = full_stripe_logical +
  1447							(i * BTRFS_STRIPE_LEN);
  1448	
  1449				if (logical >= data_stripe_start &&
  1450				    logical < data_stripe_start + BTRFS_STRIPE_LEN)
  1451					break;
  1452			}
  1453	
  1454			*stripe_index = i;
  1455			*stripe_offset = (logical - full_stripe_logical) &
> 1456					 BTRFS_STRIPE_LEN_MASK;
  1457		} else {
  1458			/* The other RAID type */
  1459			*stripe_index = mirror;
  1460			*stripe_offset = 0;
  1461		}
  1462	}
  1463
David Sterba Feb. 23, 2023, 7:51 p.m. UTC | #3 
On Wed, Feb 22, 2023 at 10:30:14AM +0800, Qu Wenruo wrote:
> [BUG]
> After commit "btrfs: replace btrfs_io_context::raid_map with a fixed u64
> value", btrfs/261 fails:
> 
>  QA output created by 261
>  ERROR: there are uncorrectable errors
>  scrub failed to fix the fs for profile -m raid5 -d raid5
>  ERROR: there are uncorrectable errors
>  scrub failed to fix the fs for profile -m raid6 -d raid6
>  Silence is golden
> 
> [CAUSE]
> In commit "btrfs: replace btrfs_io_context::raid_map with a fixed u64
> value", there is a call site using raid_map[i]:
> 
> 		*stripe_offset = logical - raid_map[i];
> 
> That location is to calculate the offset inside the stripe.
> But unfortunately the offending commit is using a wrong value:
> 
> 		*stripe_offset = logical - full_stripe_logical;
> 
> The above line is change the behavior to "logical - raid_map[0]", not
> "logical - raid_map[i]".
> 
> Thus causing wrong offset returned to the caller for raid56 replace.
> 
> [FIX]
> Thankfully the call site itself doesn't really need to access
> raid_map[i], since what we need is the offset inside the stripe.
> 
> So we can use BTRFS_STRIPE_LEN_MASK to calculate the offset inside the
> stripe.
> 
> Please fold this one into the offending commit "btrfs: replace
> btrfs_io_context::raid_map with a fixed u64 value".

Folded, thanks.
diff mbox series

Patch

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 8961dceae18b..26c6ea41821a 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1447,7 +1447,8 @@  static inline void scrub_stripe_index_and_offset(u64 logical, u64 map_type,
 		}
 
 		*stripe_index = i;
-		*stripe_offset = logical - full_stripe_logical;
+		*stripe_offset = (logical - full_stripe_logical) &
+				 BTRFS_STRIPE_LEN_MASK;
 	} else {
 		/* The other RAID type */
 		*stripe_index = mirror;