On Thursday July 10, maan@xxxxxxxxxxxxxxx wrote:
> Rename it to sb_start to make sure all users have been converted.
Good idea.
> @@ -1047,22 +1045,21 @@ static int super_1_load(mdk_rdev_t *rdev, mdk_rdev_t *refdev, int minor_version)
> */
> switch(minor_version) {
> case 0:
> - sb_offset = rdev->bdev->bd_inode->i_size >> 9;
> - sb_offset -= 8*2;
> - sb_offset &= ~(sector_t)(4*2-1);
> + sb_start = rdev->bdev->bd_inode->i_size >> 9;
> + sb_start -= 8*2;
> + sb_start &= ~(sector_t)(4*2-1);
> /* convert from sectors to K */
> - sb_offset /= 2;
> break;
That "convert.." comment can go.
>
> if (minor_version
> - && rdev->data_offset < sb_offset + (rdev->sb_size/512))
> + && rdev->data_offset < sb_start / 2 + (rdev->sb_size/512))
> return -EINVAL;
That doesn't look right.
rdev->data_offset is in sectors.
rdev->sb_size/512 is in sectors.
sb_offset is in K.
We had a bug there. Better fix it by dropping the "/ 2".
> @@ -3431,16 +3428,16 @@ static int do_md_run(mddev_t * mddev)
> * We don't want the data to overlap the metadata,
> * Internal Bitmap issues has handled elsewhere.
> */
> - if (rdev->data_offset < rdev->sb_offset) {
> + if (rdev->data_offset < rdev->sb_start / 2) {
And another bug. We don't want the "/ 2". data_offset was always
sectors, and now sb_offset is too.
That's two bugs we've found by doing this conversion - thanks!
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
