[PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing

Sasha Levin posted 238 patches 1 year, 8 months ago
[PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing
Posted by Sasha Levin 1 year, 8 months ago
From: Christoph Hellwig <hch@lst.de>

[ Upstream commit 118cf084adb3964d06e1667cf7d702e56e5cd2c5 ]

Implement the ->set_read_only method instead of parsing the actual
ioctl command.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Stable-dep-of: 9674f54e41ff ("md: Don't clear MD_CLOSING when the raid is about to stop")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/md/md.c | 62 ++++++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 03d2e31dda2f6..d6f12338cb989 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7536,7 +7536,6 @@ static inline bool md_ioctl_valid(unsigned int cmd)
 {
 	switch (cmd) {
 	case ADD_NEW_DISK:
-	case BLKROSET:
 	case GET_ARRAY_INFO:
 	case GET_BITMAP_FILE:
 	case GET_DISK_INFO:
@@ -7563,7 +7562,6 @@ static int md_ioctl(struct block_device *bdev, fmode_t mode,
 	int err = 0;
 	void __user *argp = (void __user *)arg;
 	struct mddev *mddev = NULL;
-	int ro;
 	bool did_set_md_closing = false;
 
 	if (!md_ioctl_valid(cmd))
@@ -7746,35 +7744,6 @@ static int md_ioctl(struct block_device *bdev, fmode_t mode,
 			goto unlock;
 		}
 		break;
-
-	case BLKROSET:
-		if (get_user(ro, (int __user *)(arg))) {
-			err = -EFAULT;
-			goto unlock;
-		}
-		err = -EINVAL;
-
-		/* if the bdev is going readonly the value of mddev->ro
-		 * does not matter, no writes are coming
-		 */
-		if (ro)
-			goto unlock;
-
-		/* are we are already prepared for writes? */
-		if (mddev->ro != 1)
-			goto unlock;
-
-		/* transitioning to readauto need only happen for
-		 * arrays that call md_write_start
-		 */
-		if (mddev->pers) {
-			err = restart_array(mddev);
-			if (err == 0) {
-				mddev->ro = 2;
-				set_disk_ro(mddev->gendisk, 0);
-			}
-		}
-		goto unlock;
 	}
 
 	/*
@@ -7868,6 +7837,36 @@ static int md_compat_ioctl(struct block_device *bdev, fmode_t mode,
 }
 #endif /* CONFIG_COMPAT */
 
+static int md_set_read_only(struct block_device *bdev, bool ro)
+{
+	struct mddev *mddev = bdev->bd_disk->private_data;
+	int err;
+
+	err = mddev_lock(mddev);
+	if (err)
+		return err;
+
+	if (!mddev->raid_disks && !mddev->external) {
+		err = -ENODEV;
+		goto out_unlock;
+	}
+
+	/*
+	 * Transitioning to read-auto need only happen for arrays that call
+	 * md_write_start and which are not ready for writes yet.
+	 */
+	if (!ro && mddev->ro == 1 && mddev->pers) {
+		err = restart_array(mddev);
+		if (err)
+			goto out_unlock;
+		mddev->ro = 2;
+	}
+
+out_unlock:
+	mddev_unlock(mddev);
+	return err;
+}
+
 static int md_open(struct block_device *bdev, fmode_t mode)
 {
 	/*
@@ -7944,6 +7943,7 @@ const struct block_device_operations md_fops =
 #endif
 	.getgeo		= md_getgeo,
 	.check_events	= md_check_events,
+	.set_read_only	= md_set_read_only,
 };
 
 static int md_thread(void *arg)
-- 
2.43.0
Re: [PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing
Posted by Christoph Hellwig 1 year, 8 months ago
How did we end up backporting all these block layer API changes?
Re: [PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing
Posted by Sasha Levin 1 year, 8 months ago
On Mon, Mar 25, 2024 at 02:04:35AM +0100, Christoph Hellwig wrote:
>How did we end up backporting all these block layer API changes?

They were brought in as a dependency for 9674f54e41ff ("md: Don't clear
MD_CLOSING when the raid is about to stop").

It's possible to work around bringing them during backport, but I
preferred to bring the dependencies instead.

-- 
Thanks,
Sasha
Re: [PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing
Posted by Christoph Hellwig 1 year, 8 months ago
On Mon, Mar 25, 2024 at 07:26:43AM -0400, Sasha Levin wrote:
> On Mon, Mar 25, 2024 at 02:04:35AM +0100, Christoph Hellwig wrote:
>> How did we end up backporting all these block layer API changes?
>
> They were brought in as a dependency for 9674f54e41ff ("md: Don't clear
> MD_CLOSING when the raid is about to stop").
>
> It's possible to work around bringing them during backport, but I
> preferred to bring the dependencies instead.

I really don't see what these giant backports bring us.  If people
want all the changes they'd just be better off on a modern kernel
rather than these frankenkernels.  The automatic backporting is
gettind way out of hand.  If the MD maintainers want
9674f54e41ff, maybe they can send a tailor made backport?
Re: [PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing
Posted by Song Liu 1 year, 8 months ago
Hi Li Nan,

Could you please look into this (back port 9674f54e41ff to older stable
kernels)? If there is no clean back port, I would recommend we not do
the back port.

Thanks,
Song

On Tue, Mar 26, 2024 at 12:40 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Mar 25, 2024 at 07:26:43AM -0400, Sasha Levin wrote:
> > On Mon, Mar 25, 2024 at 02:04:35AM +0100, Christoph Hellwig wrote:
> >> How did we end up backporting all these block layer API changes?
> >
> > They were brought in as a dependency for 9674f54e41ff ("md: Don't clear
> > MD_CLOSING when the raid is about to stop").
> >
> > It's possible to work around bringing them during backport, but I
> > preferred to bring the dependencies instead.
>
> I really don't see what these giant backports bring us.  If people
> want all the changes they'd just be better off on a modern kernel
> rather than these frankenkernels.  The automatic backporting is
> gettind way out of hand.  If the MD maintainers want
> 9674f54e41ff, maybe they can send a tailor made backport?
Re: [PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing
Posted by Li Nan 1 year, 8 months ago

在 2024/3/26 16:46, Song Liu 写道:
> Hi Li Nan,
> 
> Could you please look into this (back port 9674f54e41ff to older stable
> kernels)? If there is no clean back port, I would recommend we not do
> the back port.
> 

There are some conflicts to back port, which are not related to the
modification of this patch. If necessary, let me know and I can adapt and
send it :)

> Thanks,
> Song
> 
> On Tue, Mar 26, 2024 at 12:40 AM Christoph Hellwig <hch@lst.de> wrote:
>>
>> On Mon, Mar 25, 2024 at 07:26:43AM -0400, Sasha Levin wrote:
>>> On Mon, Mar 25, 2024 at 02:04:35AM +0100, Christoph Hellwig wrote:
>>>> How did we end up backporting all these block layer API changes?
>>>
>>> They were brought in as a dependency for 9674f54e41ff ("md: Don't clear
>>> MD_CLOSING when the raid is about to stop").
>>>
>>> It's possible to work around bringing them during backport, but I
>>> preferred to bring the dependencies instead.
>>
>> I really don't see what these giant backports bring us.  If people
>> want all the changes they'd just be better off on a modern kernel
>> rather than these frankenkernels.  The automatic backporting is
>> gettind way out of hand.  If the MD maintainers want
>> 9674f54e41ff, maybe they can send a tailor made backport?
> 
> .

-- 
Thanks,
Nan

Re: [PATCH 5.10 033/238] md: implement ->set_read_only to hook into BLKROSET processing
Posted by Song Liu 1 year, 8 months ago
On Tue, Mar 26, 2024 at 6:18 PM Li Nan <linan666@huaweicloud.com> wrote:
>
> 在 2024/3/26 16:46, Song Liu 写道:
> > Hi Li Nan,
> >
> > Could you please look into this (back port 9674f54e41ff to older stable
> > kernels)? If there is no clean back port, I would recommend we not do
> > the back port.
> >
>
> There are some conflicts to back port, which are not related to the
> modification of this patch. If necessary, let me know and I can adapt and
> send it :)

I think it is not necessary to back port this to older kernels with conflicts.
The error case exists theoretically. But we never got a bug report for it.

Thanks,
Song