[PATCH] md/md-bitmap: bitmap pages leaked after recovery

[PATCH] md/md-bitmap: bitmap pages leaked after recovery

[Nigel Croxon]

The bitmap pages leaked after recovery if successful writes occur
concurrently during array degradation/recovery.

We performed write I/Os to an MD RAID1 array while it was in a degraded
state or actively undergoing recovery. Even after the array recovery
successfully reached 100%, the disk status returned to normal ([UU]),
and all I/Os completely stopped, the number of bitmap pages
(bitmap: n/N pages in /proc/mdstat) never dropped back to 0, resulting
in a persistent memory leak of bitmap pages.

Bitmap pages remain permanently allocated in the kernel memory even after
the array has fully returned to a healthy state, leading to a kernel
memory leak.

When a disk is re-added and recovery starts, MD_RECOVERY_RUNNING is set
in mddev->recovery.  The else if condition evaluates to false for normal
concurrent writes during recovery. NEEDED_MASK is never applied to these
blocks.

Fixes: 4f0e7d0e03b7
md/md-bitmap: remove the last parameter for bimtap_ops->endwrite()
This is becauses removing success caused bitmap_endwrite()
to treat mddev->degraded as a blanket trigger for NEEDED_MASK.

Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
---
  drivers/md/md-bitmap.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 1d4a050dab3a..b8be0d079780 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -1756,7 +1756,14 @@ static void bitmap_end_write(struct mddev *mddev, 
sector_t offset,
                  sysfs_notify_dirent_safe(
                          bitmap->sysfs_can_clear);
              }
-        } else if (!NEEDED(*bmc)) {
+        /*
+         * Do not set NEEDED_MASK if recovery is actively running.
+         * Writes completing during recovery/resync are already covered by
+         * the recovery thread and setting NEEDED_MASK will cause orphaned
+         * bitmap flags and memory leaks after recovery finishes.
+         */
+        } else if (!test_bit(MD_RECOVERY_RUNNING,
+                &bitmap->mddev->recovery) && !NEEDED(*bmc)) {
              *bmc |= NEEDED_MASK;
          }

-- 
2.47.3

Re: [PATCH] md/md-bitmap: bitmap pages leaked after recovery

[Nigel Croxon]

On 8/7/26 2:55 PM, Nigel Croxon wrote:
> The bitmap pages leaked after recovery if successful writes occur
> concurrently during array degradation/recovery.
>
> We performed write I/Os to an MD RAID1 array while it was in a degraded
> state or actively undergoing recovery. Even after the array recovery
> successfully reached 100%, the disk status returned to normal ([UU]),
> and all I/Os completely stopped, the number of bitmap pages
> (bitmap: n/N pages in /proc/mdstat) never dropped back to 0, resulting
> in a persistent memory leak of bitmap pages.
>
> Bitmap pages remain permanently allocated in the kernel memory even after
> the array has fully returned to a healthy state, leading to a kernel
> memory leak.
>
> When a disk is re-added and recovery starts, MD_RECOVERY_RUNNING is set
> in mddev->recovery.  The else if condition evaluates to false for normal
> concurrent writes during recovery. NEEDED_MASK is never applied to these
> blocks.
>
> Fixes: 4f0e7d0e03b7
> md/md-bitmap: remove the last parameter for bimtap_ops->endwrite()
> This is becauses removing success caused bitmap_endwrite()
> to treat mddev->degraded as a blanket trigger for NEEDED_MASK.
>
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
> ---
>  drivers/md/md-bitmap.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 1d4a050dab3a..b8be0d079780 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -1756,7 +1756,14 @@ static void bitmap_end_write(struct mddev 
> *mddev, sector_t offset,
>                  sysfs_notify_dirent_safe(
>                          bitmap->sysfs_can_clear);
>              }
> -        } else if (!NEEDED(*bmc)) {
> +        /*
> +         * Do not set NEEDED_MASK if recovery is actively running.
> +         * Writes completing during recovery/resync are already 
> covered by
> +         * the recovery thread and setting NEEDED_MASK will cause 
> orphaned
> +         * bitmap flags and memory leaks after recovery finishes.
> +         */
> +        } else if (!test_bit(MD_RECOVERY_RUNNING,
> +                &bitmap->mddev->recovery) && !NEEDED(*bmc)) {
>              *bmc |= NEEDED_MASK;
>          }
>
Looking for review and feedback...

Re: [PATCH] md/md-bitmap: bitmap pages leaked after recovery

[yu kuai]

Hi,

在 2026/8/8 2:55, Nigel Croxon 写道:
> The bitmap pages leaked after recovery if successful writes occur
> concurrently during array degradation/recovery.
>
> We performed write I/Os to an MD RAID1 array while it was in a degraded
> state or actively undergoing recovery. Even after the array recovery
> successfully reached 100%, the disk status returned to normal ([UU]),
> and all I/Os completely stopped, the number of bitmap pages
> (bitmap: n/N pages in /proc/mdstat) never dropped back to 0, resulting
> in a persistent memory leak of bitmap pages.
>
> Bitmap pages remain permanently allocated in the kernel memory even after
> the array has fully returned to a healthy state, leading to a kernel
> memory leak.
>
> When a disk is re-added and recovery starts, MD_RECOVERY_RUNNING is set
> in mddev->recovery.  The else if condition evaluates to false for normal
> concurrent writes during recovery. NEEDED_MASK is never applied to these
> blocks.
>
> Fixes: 4f0e7d0e03b7
> md/md-bitmap: remove the last parameter for bimtap_ops->endwrite()
> This is becauses removing success caused bitmap_endwrite()
> to treat mddev->degraded as a blanket trigger for NEEDED_MASK.
>
> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
> ---
>  drivers/md/md-bitmap.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 1d4a050dab3a..b8be0d079780 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -1756,7 +1756,14 @@ static void bitmap_end_write(struct mddev 
> *mddev, sector_t offset,
>                  sysfs_notify_dirent_safe(
>                          bitmap->sysfs_can_clear);
>              }
> -        } else if (!NEEDED(*bmc)) {
> +        /*
> +         * Do not set NEEDED_MASK if recovery is actively running.
> +         * Writes completing during recovery/resync are already 
> covered by
> +         * the recovery thread and setting NEEDED_MASK will cause 
> orphaned
> +         * bitmap flags and memory leaks after recovery finishes.
> +         */
> +        } else if (!test_bit(MD_RECOVERY_RUNNING,
> +                &bitmap->mddev->recovery) && !NEEDED(*bmc)) {

This looks incorrect, MD_RECOVERY_RUNNING doesn't represent recovery is running.

And I'll prefer to add the success parameter back, and check it before setting
NEEDED_MASK, since this case is missing in the original patch.

BTW, no patch can be applied until the next merge window, I'm waiting for Jens
to create the block-7.3 branch.

>      *bmc |= NEEDED_MASK;
>          }
>
-- 
Thanks,
Kuai