Re: [PATCH] md: do not _put wrong device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

在 2023/09/11 18:56, Mariusz Tkaczyk 写道:
Put the device which has been _get with previous md_seq_next call.
Add guard for improper mddev_put usage(). It shouldn't be called if
there are less than 1 for mddev->active.

I didn't convert atomic_t to refcount_t because refcount warns when 0 is
achieved which is likely to happen for mddev->active.

It fixes [1], I'm unable to reproduce this issue now.


Please add a fix tag:

Fixes: 12a6caf27324 ("md: only delete entries from all_mddevs when the disk is freed")

[1]: https://bugzilla.kernel.org/show_bug.cgi?id=217798
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx>
---

There is md_seq cleanup proposed by Yu, this one should be merged
first, because it is low risk fix for particular regression.

This is not complete fix for the problem, we need to prevent new open
in the middle of removal, I will propose patchset separately.

  drivers/md/md.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0fe7ab6e8ab9..ffae02406175 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -618,6 +618,12 @@ static void mddev_delayed_delete(struct work_struct *ws);
void mddev_put(struct mddev *mddev)
  {
+	/* Guard for ambiguous put. */
+	if (unlikely(atomic_read(&mddev->active) < 1)) {
+		pr_warn("%s: active refcount is lower than 1\n", mdname(mddev));
+		return;
+	}
+
  	if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
  		return;
  	if (!mddev->raid_disks && list_empty(&mddev->disks) &&
@@ -8250,8 +8256,7 @@ static void *md_seq_next(struct seq_file *seq, void *v, loff_t *pos)
  		next_mddev = list_entry(tmp, struct mddev, all_mddevs);
  		if (mddev_get(next_mddev))
  			break;
-		mddev = next_mddev;
-		tmp = mddev->all_mddevs.next;
+		tmp = next_mddev->all_mddevs.next;
  	}
  	spin_unlock(&all_mddevs_lock);

And looks like commit 12a6caf27324 wants to mddev_put(to_put), and with
your change, 'to_put' is not needed as well, can you also remove the
local variable 'to_put'?

Thanks,
Kuai




[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux