[PATCH] md: change bitmap offset verification in write_sb_page

[PATCH] md: change bitmap offset verification in write_sb_page

[Jakub Radtke]

From: Jakub Radtke <jakub.radtke@intel.com>

Removes the code that is correct only for the native metadata.
Write-intent bitmap support for the other metadata formats is blocked.

rdev->sb_start is used in the calculations.
The sb_start is only set and used for native metadata format, and
the bitmap offset check will always fail if it is not set.

In the case of external metadata, the bitmap can be placed in various
places e.g. like the PPL between two volumes (the boundary checks are
performed on the sysfs level and in the mdadm).

Signed-off-by: Jakub Radtke <jakub.radtke@linux.intel.com>
---
 drivers/md/md-bitmap.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 200c5d0f08bf..a78b15df4d82 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -236,14 +236,6 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
 		 */
 		if (mddev->external) {
 			/* Bitmap could be anywhere. */
-			if (rdev->sb_start + offset + (page->index
-						       * (PAGE_SIZE/512))
-			    > rdev->data_offset
-			    &&
-			    rdev->sb_start + offset
-			    < (rdev->data_offset + mddev->dev_sectors
-			     + (PAGE_SIZE/512)))
-				goto bad_alignment;
 		} else if (offset < 0) {
 			/* DATA  BITMAP METADATA  */
 			if (offset
-- 
2.17.1

Re: [PATCH] md: change bitmap offset verification in write_sb_page

[Song Liu]

On Tue, Jan 5, 2021 at 7:06 AM Jakub Radtke
<jakub.radtke@linux.intel.com> wrote:
>
> From: Jakub Radtke <jakub.radtke@intel.com>
>
> Removes the code that is correct only for the native metadata.
> Write-intent bitmap support for the other metadata formats is blocked.
>
> rdev->sb_start is used in the calculations.
> The sb_start is only set and used for native metadata format, and
> the bitmap offset check will always fail if it is not set.

Can we use different logic for native and other metadata, so that we can
keep the check for native metadata? Maybe we can use the combination
of mddev->major_version, mddev->minor_version, and rdev->sb_start?

Thanks,
Song

>
> In the case of external metadata, the bitmap can be placed in various
> places e.g. like the PPL between two volumes (the boundary checks are
> performed on the sysfs level and in the mdadm).
>
> Signed-off-by: Jakub Radtke <jakub.radtke@linux.intel.com>
> ---
>  drivers/md/md-bitmap.c | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 200c5d0f08bf..a78b15df4d82 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -236,14 +236,6 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
>                  */
>                 if (mddev->external) {
>                         /* Bitmap could be anywhere. */
> -                       if (rdev->sb_start + offset + (page->index
> -                                                      * (PAGE_SIZE/512))
> -                           > rdev->data_offset
> -                           &&
> -                           rdev->sb_start + offset
> -                           < (rdev->data_offset + mddev->dev_sectors
> -                            + (PAGE_SIZE/512)))
> -                               goto bad_alignment;
>                 } else if (offset < 0) {
>                         /* DATA  BITMAP METADATA  */
>                         if (offset
> --
> 2.17.1
>

Re: [PATCH] md: change bitmap offset verification in write_sb_page

[Radtke, Jakub]

Please ignore the patch.
I see that the calculation is working for the other metadata formats too.
The problem was related to my environment, and I misinterpret the 
calculation in write_sb_page, which correctly reports the error.
> On Tue, Jan 5, 2021 at 7:06 AM Jakub Radtke
> <jakub.radtke@linux.intel.com> wrote:
>> From: Jakub Radtke <jakub.radtke@intel.com>
>>
>> Removes the code that is correct only for the native metadata.
>> Write-intent bitmap support for the other metadata formats is blocked.
>>
>> rdev->sb_start is used in the calculations.
>> The sb_start is only set and used for native metadata format, and
>> the bitmap offset check will always fail if it is not set.
> Can we use different logic for native and other metadata, so that we can
> keep the check for native metadata? Maybe we can use the combination
> of mddev->major_version, mddev->minor_version, and rdev->sb_start?
>
> Thanks,
> Song
>
>> In the case of external metadata, the bitmap can be placed in various
>> places e.g. like the PPL between two volumes (the boundary checks are
>> performed on the sysfs level and in the mdadm).
>>
>> Signed-off-by: Jakub Radtke <jakub.radtke@linux.intel.com>
>> ---
>>   drivers/md/md-bitmap.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 200c5d0f08bf..a78b15df4d82 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -236,14 +236,6 @@ static int write_sb_page(struct bitmap *bitmap, struct page *page, int wait)
>>                   */
>>                  if (mddev->external) {
>>                          /* Bitmap could be anywhere. */
>> -                       if (rdev->sb_start + offset + (page->index
>> -                                                      * (PAGE_SIZE/512))
>> -                           > rdev->data_offset
>> -                           &&
>> -                           rdev->sb_start + offset
>> -                           < (rdev->data_offset + mddev->dev_sectors
>> -                            + (PAGE_SIZE/512)))
>> -                               goto bad_alignment;
>>                  } else if (offset < 0) {
>>                          /* DATA  BITMAP METADATA  */
>>                          if (offset
>> --
>> 2.17.1
>>