md: Added journal count to md_u.h

[PATCH] md: Added journal count to md_u.h

Posted by Abhinav Jain 1 year, 10 months ago

This patch addresses TODO in "./drivers/md/md.c:6752".
Add journal struct member to mdu_array_info_t in "md_u.h".
Add the journal field in get_array_info function in "md.c".
Need feedback on if the update_array_info function needs to be updated.

Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com>
---
 drivers/md/md.c                | 8 ++++----
 include/uapi/linux/raid/md_u.h | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index e575e74aabf5..4958d6e21923 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6735,10 +6735,10 @@ static int get_version(void __user *arg)
 static int get_array_info(struct mddev *mddev, void __user *arg)
 {
 	mdu_array_info_t info;
-	int nr,working,insync,failed,spare;
+	int nr, working, insync, failed, spare, journal;
 	struct md_rdev *rdev;
 
-	nr = working = insync = failed = spare = 0;
+	nr = working = insync = failed = spare = journal = 0;
 	rcu_read_lock();
 	rdev_for_each_rcu(rdev, mddev) {
 		nr++;
@@ -6749,8 +6749,7 @@ static int get_array_info(struct mddev *mddev, void __user *arg)
 			if (test_bit(In_sync, &rdev->flags))
 				insync++;
 			else if (test_bit(Journal, &rdev->flags))
-				/* TODO: add journal count to md_u.h */
-				;
+				journal++;
 			else
 				spare++;
 		}
@@ -6769,6 +6768,7 @@ static int get_array_info(struct mddev *mddev, void __user *arg)
 	info.raid_disks    = mddev->raid_disks;
 	info.md_minor      = mddev->md_minor;
 	info.not_persistent= !mddev->persistent;
+	info.journal       = journal;
 
 	info.utime         = clamp_t(time64_t, mddev->utime, 0, U32_MAX);
 	info.state         = 0;
diff --git a/include/uapi/linux/raid/md_u.h b/include/uapi/linux/raid/md_u.h
index 7be89a4906e7..485befa2205d 100644
--- a/include/uapi/linux/raid/md_u.h
+++ b/include/uapi/linux/raid/md_u.h
@@ -84,6 +84,7 @@ typedef struct mdu_array_info_s {
 	int raid_disks;
 	int md_minor;
 	int not_persistent;
+	int journal;
 
 	/*
 	 * Generic state information
-- 
2.34.1

Re: [PATCH] md: Added journal count to md_u.h

Posted by Mariusz Tkaczyk 1 year, 10 months ago

On Mon, 15 Apr 2024 16:08:18 +0000
Abhinav Jain <jain.abhinav177@gmail.com> wrote:

> This patch addresses TODO in "./drivers/md/md.c:6752".
> Add journal struct member to mdu_array_info_t in "md_u.h".
> Add the journal field in get_array_info function in "md.c".
> Need feedback on if the update_array_info function needs to be updated.
> 
> Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com>
> ---

Hello,
The field you added here is unused. I believe it works but I cannot determine
that as the filed you added in not exposed to user (probably --detail is right
place). Please do some tests and include the example test in commit description.
Please also explain why you see it useful.

For the question you asked, having it in update_array_info requires possibility
to modify/set from userspace, so user interface and/or corresponding metadata
property is needed.

Please explain why it would be useful to have possibility to set it from
userspace, what are the benefits.

Thanks,
Mariusz

Re: [PATCH] md: Added journal count to md_u.h

Posted by Paul Menzel 1 year, 10 months ago

Dear Abhinav,


Thank you for your patch. Some formal nits. Could you please use 
imperative mood in the commit message summary/title?

Am 15.04.24 um 18:08 schrieb Abhinav Jain:
> This patch addresses TODO in "./drivers/md/md.c:6752".

Maybe quote the to-do?

> Add journal struct member to mdu_array_info_t in "md_u.h".
> Add the journal field in get_array_info function in "md.c".

I wouldn’t break the line, just because a sentence ends, and use 75 
characters per line. Or make it a list/enumeration.

> Need feedback on if the update_array_info function needs to be updated.

If you need feedback, please tag the patch, that means, add [RFC] to the 
subject/summary/title.

How did you test this?


Kind regards,

Paul


> Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com>
> ---
>   drivers/md/md.c                | 8 ++++----
>   include/uapi/linux/raid/md_u.h | 1 +
>   2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index e575e74aabf5..4958d6e21923 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6735,10 +6735,10 @@ static int get_version(void __user *arg)
>   static int get_array_info(struct mddev *mddev, void __user *arg)
>   {
>   	mdu_array_info_t info;
> -	int nr,working,insync,failed,spare;
> +	int nr, working, insync, failed, spare, journal;
>   	struct md_rdev *rdev;
>   
> -	nr = working = insync = failed = spare = 0;
> +	nr = working = insync = failed = spare = journal = 0;
>   	rcu_read_lock();
>   	rdev_for_each_rcu(rdev, mddev) {
>   		nr++;
> @@ -6749,8 +6749,7 @@ static int get_array_info(struct mddev *mddev, void __user *arg)
>   			if (test_bit(In_sync, &rdev->flags))
>   				insync++;
>   			else if (test_bit(Journal, &rdev->flags))
> -				/* TODO: add journal count to md_u.h */
> -				;
> +				journal++;
>   			else
>   				spare++;
>   		}
> @@ -6769,6 +6768,7 @@ static int get_array_info(struct mddev *mddev, void __user *arg)
>   	info.raid_disks    = mddev->raid_disks;
>   	info.md_minor      = mddev->md_minor;
>   	info.not_persistent= !mddev->persistent;
> +	info.journal       = journal;
>   
>   	info.utime         = clamp_t(time64_t, mddev->utime, 0, U32_MAX);
>   	info.state         = 0;
> diff --git a/include/uapi/linux/raid/md_u.h b/include/uapi/linux/raid/md_u.h
> index 7be89a4906e7..485befa2205d 100644
> --- a/include/uapi/linux/raid/md_u.h
> +++ b/include/uapi/linux/raid/md_u.h
> @@ -84,6 +84,7 @@ typedef struct mdu_array_info_s {
>   	int raid_disks;
>   	int md_minor;
>   	int not_persistent;
> +	int journal;
>   
>   	/*
>   	 * Generic state information