Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
From:   Yu Kuai <yukuai1@huaweicloud.com>
To:     song@kernel.org
Cc:     linux-raid@vger.kernel.org, linux-kernel@vger.kernel.org,
        yukuai3@huawei.com, yukuai1@huaweicloud.com, yi.zhang@huawei.com,
        yangerkun@huawei.com
Subject: [PATCH -next v2 1/6] md: remove useless debug code to print configuration
Date:   Sat, 21 Oct 2023 18:20:54 +0800
Message-Id: <20231021102059.3198284-2-yukuai1@huaweicloud.com>
In-Reply-To: <20231021102059.3198284-1-yukuai1@huaweicloud.com>
References: <20231021102059.3198284-1-yukuai1@huaweicloud.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Yu Kuai <yukuai3@huawei.com>

One the one hand, print_conf() can be called without grabbing
'reconfig_mtuex' and current rcu protection to access rdev through 'conf'
is not safe. Fortunately, there is a separate rcu protection to access
rdev from 'mddev->disks', and rdev is always removed from 'conf' before
'mddev->disks'.

On the other hand, print_conf() is just used for debug,
and user can always grab such information(/proc/mdstat and mdadm).

There is no need to always enable this debug and try to fix misuse rcu
protection for accessing rdev from 'conf', hence remove print_conf().

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/raid1.c  | 28 ----------------------------
 drivers/md/raid10.c | 29 -----------------------------
 drivers/md/raid5.c  | 34 ----------------------------------
 3 files changed, 91 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 35d12948e0a9..c13088eae401 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1679,30 +1679,6 @@ static void raid1_error(struct mddev *mddev, struct md_rdev *rdev)
 		mdname(mddev), conf->raid_disks - mddev->degraded);
 }
 
-static void print_conf(struct r1conf *conf)
-{
-	int i;
-
-	pr_debug("RAID1 conf printout:\n");
-	if (!conf) {
-		pr_debug("(!conf)\n");
-		return;
-	}
-	pr_debug(" --- wd:%d rd:%d\n", conf->raid_disks - conf->mddev->degraded,
-		 conf->raid_disks);
-
-	rcu_read_lock();
-	for (i = 0; i < conf->raid_disks; i++) {
-		struct md_rdev *rdev = rcu_dereference(conf->mirrors[i].rdev);
-		if (rdev)
-			pr_debug(" disk %d, wo:%d, o:%d, dev:%pg\n",
-				 i, !test_bit(In_sync, &rdev->flags),
-				 !test_bit(Faulty, &rdev->flags),
-				 rdev->bdev);
-	}
-	rcu_read_unlock();
-}
-
 static void close_sync(struct r1conf *conf)
 {
 	int idx;
@@ -1763,7 +1739,6 @@ static int raid1_spare_active(struct mddev *mddev)
 	mddev->degraded -= count;
 	spin_unlock_irqrestore(&conf->device_lock, flags);
 
-	print_conf(conf);
 	return count;
 }
 
@@ -1829,7 +1804,6 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
 		rcu_assign_pointer(p[conf->raid_disks].rdev, rdev);
 	}
 
-	print_conf(conf);
 	return err;
 }
 
@@ -1846,7 +1820,6 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
 	if (rdev != p->rdev)
 		p = conf->mirrors + conf->raid_disks + number;
 
-	print_conf(conf);
 	if (rdev == p->rdev) {
 		if (test_bit(In_sync, &rdev->flags) ||
 		    atomic_read(&rdev->nr_pending)) {
@@ -1902,7 +1875,6 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
 	}
 abort:
 
-	print_conf(conf);
 	return err;
 }
 
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index a5927e98dc67..4b5f34f320c8 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2059,31 +2059,6 @@ static void raid10_error(struct mddev *mddev, struct md_rdev *rdev)
 		mdname(mddev), conf->geo.raid_disks - mddev->degraded);
 }
 
-static void print_conf(struct r10conf *conf)
-{
-	int i;
-	struct md_rdev *rdev;
-
-	pr_debug("RAID10 conf printout:\n");
-	if (!conf) {
-		pr_debug("(!conf)\n");
-		return;
-	}
-	pr_debug(" --- wd:%d rd:%d\n", conf->geo.raid_disks - conf->mddev->degraded,
-		 conf->geo.raid_disks);
-
-	/* This is only called with ->reconfix_mutex held, so
-	 * rcu protection of rdev is not needed */
-	for (i = 0; i < conf->geo.raid_disks; i++) {
-		rdev = conf->mirrors[i].rdev;
-		if (rdev)
-			pr_debug(" disk %d, wo:%d, o:%d, dev:%pg\n",
-				 i, !test_bit(In_sync, &rdev->flags),
-				 !test_bit(Faulty, &rdev->flags),
-				 rdev->bdev);
-	}
-}
-
 static void close_sync(struct r10conf *conf)
 {
 	wait_barrier(conf, false);
@@ -2136,7 +2111,6 @@ static int raid10_spare_active(struct mddev *mddev)
 	mddev->degraded -= count;
 	spin_unlock_irqrestore(&conf->device_lock, flags);
 
-	print_conf(conf);
 	return count;
 }
 
@@ -2207,7 +2181,6 @@ static int raid10_add_disk(struct mddev *mddev, struct md_rdev *rdev)
 		rcu_assign_pointer(p->replacement, rdev);
 	}
 
-	print_conf(conf);
 	return err;
 }
 
@@ -2219,7 +2192,6 @@ static int raid10_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
 	struct md_rdev **rdevp;
 	struct raid10_info *p;
 
-	print_conf(conf);
 	if (unlikely(number >= mddev->raid_disks))
 		return 0;
 	p = conf->mirrors + number;
@@ -2271,7 +2243,6 @@ static int raid10_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
 
 abort:
 
-	print_conf(conf);
 	return err;
 }
 
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 4207e945e8c8..27a4dce51c92 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -156,8 +156,6 @@ static int raid6_idx_to_slot(int idx, struct stripe_head *sh,
 	return slot;
 }
 
-static void print_raid5_conf (struct r5conf *conf);
-
 static int stripe_operations_active(struct stripe_head *sh)
 {
 	return sh->check_state || sh->reconstruct_state ||
@@ -7983,8 +7981,6 @@ static int raid5_run(struct mddev *mddev)
 		mddev->raid_disks-mddev->degraded, mddev->raid_disks,
 		mddev->new_layout);
 
-	print_raid5_conf(conf);
-
 	if (conf->reshape_progress != MaxSector) {
 		conf->reshape_safe = conf->reshape_progress;
 		atomic_set(&conf->reshape_stripes, 0);
@@ -8075,7 +8071,6 @@ static int raid5_run(struct mddev *mddev)
 	return 0;
 abort:
 	md_unregister_thread(mddev, &mddev->thread);
-	print_raid5_conf(conf);
 	free_conf(conf);
 	mddev->private = NULL;
 	pr_warn("md/raid:%s: failed to run raid set.\n", mdname(mddev));
@@ -8107,31 +8102,6 @@ static void raid5_status(struct seq_file *seq, struct mddev *mddev)
 	seq_printf (seq, "]");
 }
 
-static void print_raid5_conf (struct r5conf *conf)
-{
-	struct md_rdev *rdev;
-	int i;
-
-	pr_debug("RAID conf printout:\n");
-	if (!conf) {
-		pr_debug("(conf==NULL)\n");
-		return;
-	}
-	pr_debug(" --- level:%d rd:%d wd:%d\n", conf->level,
-	       conf->raid_disks,
-	       conf->raid_disks - conf->mddev->degraded);
-
-	rcu_read_lock();
-	for (i = 0; i < conf->raid_disks; i++) {
-		rdev = rcu_dereference(conf->disks[i].rdev);
-		if (rdev)
-			pr_debug(" disk %d, o:%d, dev:%pg\n",
-			       i, !test_bit(Faulty, &rdev->flags),
-			       rdev->bdev);
-	}
-	rcu_read_unlock();
-}
-
 static int raid5_spare_active(struct mddev *mddev)
 {
 	int i;
@@ -8173,7 +8143,6 @@ static int raid5_spare_active(struct mddev *mddev)
 	spin_lock_irqsave(&conf->device_lock, flags);
 	mddev->degraded = raid5_calc_degraded(conf);
 	spin_unlock_irqrestore(&conf->device_lock, flags);
-	print_raid5_conf(conf);
 	return count;
 }
 
@@ -8186,7 +8155,6 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
 	struct disk_info *p;
 	struct md_rdev *tmp;
 
-	print_raid5_conf(conf);
 	if (test_bit(Journal, &rdev->flags) && conf->log) {
 		/*
 		 * we can't wait pending write here, as this is called in
@@ -8266,7 +8234,6 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
 	clear_bit(WantReplacement, &rdev->flags);
 abort:
 
-	print_raid5_conf(conf);
 	return err;
 }
 
@@ -8348,7 +8315,6 @@ static int raid5_add_disk(struct mddev *mddev, struct md_rdev *rdev)
 		}
 	}
 out:
-	print_raid5_conf(conf);
 	return err;
 }
 
-- 
2.39.2