Re: [PATCH v5 01/16] md/raid456: covert macro define of STRIPE_* as members of struct r5conf

[Guoqing Jiang <guoqing.jiang@xxxxxxxxxxxxxxx>]

On 7/2/20 2:06 PM, Yufen Yu wrote:
> @@ -2509,10 +2532,11 @@ static void raid5_end_read_request(struct bio * bi)
>   			 */
>   			pr_info_ratelimited(
>   				"md/raid:%s: read error corrected (%lu sectors at %llu on %s)\n",
> -				mdname(conf->mddev), STRIPE_SECTORS,
> +				mdname(conf->mddev),
> +				conf->stripe_sectors,

The conf->stripe_sectors is printed with %lu format.

> ot allow a suitable chunk size */
>   		return ERR_PTR(-EINVAL);
>   
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index f90e0704bed9..e36cf71e8465 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
> @@ -472,32 +472,12 @@ struct disk_info {
>    */
>   
>   #define NR_STRIPES		256
> -#define STRIPE_SIZE		PAGE_SIZE
> -#define STRIPE_SHIFT		(PAGE_SHIFT - 9)
> -#define STRIPE_SECTORS		(STRIPE_SIZE>>9)
>   #define	IO_THRESHOLD		1
>   #define BYPASS_THRESHOLD	1
>   #define NR_HASH			(PAGE_SIZE / sizeof(struct hlist_head))
>   #define HASH_MASK		(NR_HASH - 1)
>   #define MAX_STRIPE_BATCH	8
>   

[...]

> 	return NULL;
> -}
> -
>   /* NOTE NR_STRIPE_HASH_LOCKS must remain below 64.
>    * This is because we sometimes take all the spinlocks
>    * and creating that much locking depth can cause
> @@ -574,6 +554,9 @@ struct r5conf {
>   	int			raid_disks;
>   	int			max_nr_stripes;
>   	int			min_nr_stripes;
> +	unsigned int	stripe_size;
> +	unsigned int	stripe_shift;
> +	unsigned int	stripe_sectors;

So you need to define it with "unsigned long".

Also, I am wondering if it is cleaner with something like below.

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8dea4398b191..984eea97e77c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -6918,6 +6918,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
        conf = kzalloc(sizeof(struct r5conf), GFP_KERNEL);
        if (conf == NULL)
                goto abort;
+       r5conf_set_size(conf, PAGE_SIZE);
        INIT_LIST_HEAD(&conf->free_list);
        INIT_LIST_HEAD(&conf->pending_list);
        conf->pending_data = kcalloc(PENDING_IO_MAX,
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index f90e0704bed9..04fc4c514d54 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -471,10 +471,13 @@ struct disk_info {
  * Stripe cache
  */

+static unsigned long stripe_size = PAGE_SIZE;
+static unsigned long stripe_shift = PAGE_SHIFT - 9;
+static unsigned long stripe_sectors = PAGE_SIZE>>9;
 #define NR_STRIPES             256
-#define STRIPE_SIZE            PAGE_SIZE
-#define STRIPE_SHIFT           (PAGE_SHIFT - 9)
-#define STRIPE_SECTORS         (STRIPE_SIZE>>9)
+#define STRIPE_SIZE            stripe_size
+#define STRIPE_SHIFT           stripe_shift
+#define STRIPE_SECTORS         stripe_sectors
 #define        IO_THRESHOLD            1
 #define BYPASS_THRESHOLD       1
 #define NR_HASH                        (PAGE_SIZE / sizeof(struct 
hlist_head))
@@ -574,6 +577,9 @@ struct r5conf {
        int                     raid_disks;
        int                     max_nr_stripes;
        int                     min_nr_stripes;
+       unsigned long           stripe_size;
+       unsigned long           stripe_shift;
+       unsigned long           stripe_sectors;

        /* reshape_progress is the leading edge of a 'reshape'
         * It has value MaxSector when no reshape is happening
@@ -690,6 +696,12 @@ struct r5conf {
        struct r5pending_data   *next_pending_data;
 };

+static inline void r5conf_set_size(struct r5conf *conf, unsigned long size)
+{
+       stripe_size = conf->stripe_size = size;
+       stripe_shift = conf->stripe_shift = ilog2(size) - 9;
+       stripe_sectors = conf->stripe_sectors = size >> 9;
+}

 /*
  * Our supported algorithms

Thanks,
Guoqing