Re: [PATCH 2/6] md: convert to kvmalloc

2018-09-07 Thread Kent Overstreet
On Fri, Sep 07, 2018 at 10:49:42AM -0700, Matthew Wilcox wrote:
> On Fri, Sep 07, 2018 at 12:56:31PM -0400, Kent Overstreet wrote:
> > @@ -165,7 +164,7 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> > raid5_percpu *percpu,
> >struct dma_async_tx_descriptor *tx)
> >  {
> > int disks = sh->disks;
> > -   struct page **srcs = flex_array_get(percpu->scribble, 0);
> > +   struct page **srcs = percpu->scribble;
> > int count = 0, pd_idx = sh->pd_idx, i;
> > struct async_submit_ctl submit;
> >  
> > @@ -196,8 +195,8 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> > raid5_percpu *percpu,
> > }
> >  
> > init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
> > - NULL, sh, flex_array_get(percpu->scribble, 0)
> > - + sizeof(struct page *) * (sh->disks + 2));
> > + NULL, sh, percpu->scribble +
> > + sizeof(struct page *) * (sh->disks + 2));
> 
> I think this would read better written as:
> 
>   init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
> NULL, sh, srcs + sh->disks + 2);
> 
> >  static addr_conv_t *to_addr_conv(struct stripe_head *sh,
> >  struct raid5_percpu *percpu, int i)
> >  {
> > -   void *addr;
> > -
> > -   addr = flex_array_get(percpu->scribble, i);
> > -   return addr + sizeof(struct page *) * (sh->disks + 2);
> > +   return percpu->scribble + i * percpu->scribble_obj_size +
> > +   sizeof(struct page *) * (sh->disks + 2);
> >  }
> >  
> >  /* return a pointer to the address conversion region of the scribble 
> > buffer */
> >  static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
> >  {
> > -   void *addr;
> > -
> > -   addr = flex_array_get(percpu->scribble, i);
> > -   return addr;
> > +   return percpu->scribble + i * percpu->scribble_obj_size;
> >  }
> 
> Perhaps this would be better as ...
> 
>  static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr;
> + return percpu->scribble + i * percpu->scribble_obj_size;
>  }
> 
>  static addr_conv_t *to_addr_conv(struct stripe_head *sh,
>struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr + sizeof(struct page *) * (sh->disks + 2);
> + return to_addr_page(percpu, i) + sh->disks + 2;
>  }
> 
> 
> The rest looks good.

Need some casts (to void * or addr_conv_t *) but yeah, I suppose that's a bit
cleaner.


Re: [PATCH 2/6] md: convert to kvmalloc

2018-09-07 Thread Kent Overstreet
On Fri, Sep 07, 2018 at 10:49:42AM -0700, Matthew Wilcox wrote:
> On Fri, Sep 07, 2018 at 12:56:31PM -0400, Kent Overstreet wrote:
> > @@ -165,7 +164,7 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> > raid5_percpu *percpu,
> >struct dma_async_tx_descriptor *tx)
> >  {
> > int disks = sh->disks;
> > -   struct page **srcs = flex_array_get(percpu->scribble, 0);
> > +   struct page **srcs = percpu->scribble;
> > int count = 0, pd_idx = sh->pd_idx, i;
> > struct async_submit_ctl submit;
> >  
> > @@ -196,8 +195,8 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> > raid5_percpu *percpu,
> > }
> >  
> > init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
> > - NULL, sh, flex_array_get(percpu->scribble, 0)
> > - + sizeof(struct page *) * (sh->disks + 2));
> > + NULL, sh, percpu->scribble +
> > + sizeof(struct page *) * (sh->disks + 2));
> 
> I think this would read better written as:
> 
>   init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
> NULL, sh, srcs + sh->disks + 2);
> 
> >  static addr_conv_t *to_addr_conv(struct stripe_head *sh,
> >  struct raid5_percpu *percpu, int i)
> >  {
> > -   void *addr;
> > -
> > -   addr = flex_array_get(percpu->scribble, i);
> > -   return addr + sizeof(struct page *) * (sh->disks + 2);
> > +   return percpu->scribble + i * percpu->scribble_obj_size +
> > +   sizeof(struct page *) * (sh->disks + 2);
> >  }
> >  
> >  /* return a pointer to the address conversion region of the scribble 
> > buffer */
> >  static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
> >  {
> > -   void *addr;
> > -
> > -   addr = flex_array_get(percpu->scribble, i);
> > -   return addr;
> > +   return percpu->scribble + i * percpu->scribble_obj_size;
> >  }
> 
> Perhaps this would be better as ...
> 
>  static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr;
> + return percpu->scribble + i * percpu->scribble_obj_size;
>  }
> 
>  static addr_conv_t *to_addr_conv(struct stripe_head *sh,
>struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr + sizeof(struct page *) * (sh->disks + 2);
> + return to_addr_page(percpu, i) + sh->disks + 2;
>  }
> 
> 
> The rest looks good.

Need some casts (to void * or addr_conv_t *) but yeah, I suppose that's a bit
cleaner.


Re: [PATCH 2/6] md: convert to kvmalloc

2018-09-07 Thread Matthew Wilcox
On Fri, Sep 07, 2018 at 12:56:31PM -0400, Kent Overstreet wrote:
> @@ -165,7 +164,7 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> raid5_percpu *percpu,
>  struct dma_async_tx_descriptor *tx)
>  {
>   int disks = sh->disks;
> - struct page **srcs = flex_array_get(percpu->scribble, 0);
> + struct page **srcs = percpu->scribble;
>   int count = 0, pd_idx = sh->pd_idx, i;
>   struct async_submit_ctl submit;
>  
> @@ -196,8 +195,8 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> raid5_percpu *percpu,
>   }
>  
>   init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
> -   NULL, sh, flex_array_get(percpu->scribble, 0)
> -   + sizeof(struct page *) * (sh->disks + 2));
> +   NULL, sh, percpu->scribble +
> +   sizeof(struct page *) * (sh->disks + 2));

I think this would read better written as:

init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
  NULL, sh, srcs + sh->disks + 2);

>  static addr_conv_t *to_addr_conv(struct stripe_head *sh,
>struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr + sizeof(struct page *) * (sh->disks + 2);
> + return percpu->scribble + i * percpu->scribble_obj_size +
> + sizeof(struct page *) * (sh->disks + 2);
>  }
>  
>  /* return a pointer to the address conversion region of the scribble buffer 
> */
>  static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr;
> + return percpu->scribble + i * percpu->scribble_obj_size;
>  }

Perhaps this would be better as ...

 static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr;
+   return percpu->scribble + i * percpu->scribble_obj_size;
 }

 static addr_conv_t *to_addr_conv(struct stripe_head *sh,
 struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr + sizeof(struct page *) * (sh->disks + 2);
+   return to_addr_page(percpu, i) + sh->disks + 2;
 }


The rest looks good.


Re: [PATCH 2/6] md: convert to kvmalloc

2018-09-07 Thread Matthew Wilcox
On Fri, Sep 07, 2018 at 12:56:31PM -0400, Kent Overstreet wrote:
> @@ -165,7 +164,7 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> raid5_percpu *percpu,
>  struct dma_async_tx_descriptor *tx)
>  {
>   int disks = sh->disks;
> - struct page **srcs = flex_array_get(percpu->scribble, 0);
> + struct page **srcs = percpu->scribble;
>   int count = 0, pd_idx = sh->pd_idx, i;
>   struct async_submit_ctl submit;
>  
> @@ -196,8 +195,8 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
> raid5_percpu *percpu,
>   }
>  
>   init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
> -   NULL, sh, flex_array_get(percpu->scribble, 0)
> -   + sizeof(struct page *) * (sh->disks + 2));
> +   NULL, sh, percpu->scribble +
> +   sizeof(struct page *) * (sh->disks + 2));

I think this would read better written as:

init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
  NULL, sh, srcs + sh->disks + 2);

>  static addr_conv_t *to_addr_conv(struct stripe_head *sh,
>struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr + sizeof(struct page *) * (sh->disks + 2);
> + return percpu->scribble + i * percpu->scribble_obj_size +
> + sizeof(struct page *) * (sh->disks + 2);
>  }
>  
>  /* return a pointer to the address conversion region of the scribble buffer 
> */
>  static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
>  {
> - void *addr;
> -
> - addr = flex_array_get(percpu->scribble, i);
> - return addr;
> + return percpu->scribble + i * percpu->scribble_obj_size;
>  }

Perhaps this would be better as ...

 static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr;
+   return percpu->scribble + i * percpu->scribble_obj_size;
 }

 static addr_conv_t *to_addr_conv(struct stripe_head *sh,
 struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr + sizeof(struct page *) * (sh->disks + 2);
+   return to_addr_page(percpu, i) + sh->disks + 2;
 }


The rest looks good.


[PATCH 2/6] md: convert to kvmalloc

2018-09-07 Thread Kent Overstreet
The code really just wants a big flat buffer, so just do that.

Signed-off-by: Kent Overstreet 
Cc: Shaohua Li 
Cc: linux-r...@vger.kernel.org
---
 drivers/md/raid5-ppl.c |  7 ++--
 drivers/md/raid5.c | 82 +++---
 drivers/md/raid5.h |  9 ++---
 3 files changed, 45 insertions(+), 53 deletions(-)

diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index 3a7c363265..5911810101 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -16,7 +16,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include "md.h"
@@ -165,7 +164,7 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
raid5_percpu *percpu,
   struct dma_async_tx_descriptor *tx)
 {
int disks = sh->disks;
-   struct page **srcs = flex_array_get(percpu->scribble, 0);
+   struct page **srcs = percpu->scribble;
int count = 0, pd_idx = sh->pd_idx, i;
struct async_submit_ctl submit;
 
@@ -196,8 +195,8 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
raid5_percpu *percpu,
}
 
init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
- NULL, sh, flex_array_get(percpu->scribble, 0)
- + sizeof(struct page *) * (sh->disks + 2));
+ NULL, sh, percpu->scribble +
+ sizeof(struct page *) * (sh->disks + 2));
 
if (count == 1)
tx = async_memcpy(sh->ppl_page, srcs[0], 0, 0, PAGE_SIZE,
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2031506a0e..d5603946dc 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -54,7 +54,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -1399,19 +1398,14 @@ static void ops_complete_compute(void *stripe_head_ref)
 static addr_conv_t *to_addr_conv(struct stripe_head *sh,
 struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr + sizeof(struct page *) * (sh->disks + 2);
+   return percpu->scribble + i * percpu->scribble_obj_size +
+   sizeof(struct page *) * (sh->disks + 2);
 }
 
 /* return a pointer to the address conversion region of the scribble buffer */
 static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr;
+   return percpu->scribble + i * percpu->scribble_obj_size;
 }
 
 static struct dma_async_tx_descriptor *
@@ -2240,21 +2234,23 @@ static int grow_stripes(struct r5conf *conf, int num)
  * calculate over all devices (not just the data blocks), using zeros in place
  * of the P and Q blocks.
  */
-static struct flex_array *scribble_alloc(int num, int cnt, gfp_t flags)
+static int scribble_alloc(struct raid5_percpu *percpu,
+ int num, int cnt, gfp_t flags)
 {
-   struct flex_array *ret;
-   size_t len;
+   size_t obj_size =
+   sizeof(struct page *) * (num+2) +
+   sizeof(addr_conv_t) * (num+2);
+   void *scribble;
 
-   len = sizeof(struct page *) * (num+2) + sizeof(addr_conv_t) * (num+2);
-   ret = flex_array_alloc(len, cnt, flags);
-   if (!ret)
-   return NULL;
-   /* always prealloc all elements, so no locking is required */
-   if (flex_array_prealloc(ret, 0, cnt, flags)) {
-   flex_array_free(ret);
-   return NULL;
-   }
-   return ret;
+   scribble = kvmalloc_array(cnt, obj_size, flags);
+   if (!scribble)
+   return -ENOMEM;
+
+   kvfree(percpu->scribble);
+
+   percpu->scribble = scribble;
+   percpu->scribble_obj_size = obj_size;
+   return 0;
 }
 
 static int resize_chunks(struct r5conf *conf, int new_disks, int new_sectors)
@@ -2272,23 +2268,18 @@ static int resize_chunks(struct r5conf *conf, int 
new_disks, int new_sectors)
return 0;
mddev_suspend(conf->mddev);
get_online_cpus();
+
for_each_present_cpu(cpu) {
struct raid5_percpu *percpu;
-   struct flex_array *scribble;
 
percpu = per_cpu_ptr(conf->percpu, cpu);
-   scribble = scribble_alloc(new_disks,
- new_sectors / STRIPE_SECTORS,
- GFP_NOIO);
-
-   if (scribble) {
-   flex_array_free(percpu->scribble);
-   percpu->scribble = scribble;
-   } else {
-   err = -ENOMEM;
+   err = scribble_alloc(percpu, new_disks,
+new_sectors / STRIPE_SECTORS,
+GFP_NOIO);
+   if (err)
break;
-   }
}
+
put_online_cpus();
mddev_resume(conf->mddev);

[PATCH 2/6] md: convert to kvmalloc

2018-09-07 Thread Kent Overstreet
The code really just wants a big flat buffer, so just do that.

Signed-off-by: Kent Overstreet 
Cc: Shaohua Li 
Cc: linux-r...@vger.kernel.org
---
 drivers/md/raid5-ppl.c |  7 ++--
 drivers/md/raid5.c | 82 +++---
 drivers/md/raid5.h |  9 ++---
 3 files changed, 45 insertions(+), 53 deletions(-)

diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index 3a7c363265..5911810101 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -16,7 +16,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include "md.h"
@@ -165,7 +164,7 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
raid5_percpu *percpu,
   struct dma_async_tx_descriptor *tx)
 {
int disks = sh->disks;
-   struct page **srcs = flex_array_get(percpu->scribble, 0);
+   struct page **srcs = percpu->scribble;
int count = 0, pd_idx = sh->pd_idx, i;
struct async_submit_ctl submit;
 
@@ -196,8 +195,8 @@ ops_run_partial_parity(struct stripe_head *sh, struct 
raid5_percpu *percpu,
}
 
init_async_submit(, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, tx,
- NULL, sh, flex_array_get(percpu->scribble, 0)
- + sizeof(struct page *) * (sh->disks + 2));
+ NULL, sh, percpu->scribble +
+ sizeof(struct page *) * (sh->disks + 2));
 
if (count == 1)
tx = async_memcpy(sh->ppl_page, srcs[0], 0, 0, PAGE_SIZE,
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2031506a0e..d5603946dc 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -54,7 +54,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -1399,19 +1398,14 @@ static void ops_complete_compute(void *stripe_head_ref)
 static addr_conv_t *to_addr_conv(struct stripe_head *sh,
 struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr + sizeof(struct page *) * (sh->disks + 2);
+   return percpu->scribble + i * percpu->scribble_obj_size +
+   sizeof(struct page *) * (sh->disks + 2);
 }
 
 /* return a pointer to the address conversion region of the scribble buffer */
 static struct page **to_addr_page(struct raid5_percpu *percpu, int i)
 {
-   void *addr;
-
-   addr = flex_array_get(percpu->scribble, i);
-   return addr;
+   return percpu->scribble + i * percpu->scribble_obj_size;
 }
 
 static struct dma_async_tx_descriptor *
@@ -2240,21 +2234,23 @@ static int grow_stripes(struct r5conf *conf, int num)
  * calculate over all devices (not just the data blocks), using zeros in place
  * of the P and Q blocks.
  */
-static struct flex_array *scribble_alloc(int num, int cnt, gfp_t flags)
+static int scribble_alloc(struct raid5_percpu *percpu,
+ int num, int cnt, gfp_t flags)
 {
-   struct flex_array *ret;
-   size_t len;
+   size_t obj_size =
+   sizeof(struct page *) * (num+2) +
+   sizeof(addr_conv_t) * (num+2);
+   void *scribble;
 
-   len = sizeof(struct page *) * (num+2) + sizeof(addr_conv_t) * (num+2);
-   ret = flex_array_alloc(len, cnt, flags);
-   if (!ret)
-   return NULL;
-   /* always prealloc all elements, so no locking is required */
-   if (flex_array_prealloc(ret, 0, cnt, flags)) {
-   flex_array_free(ret);
-   return NULL;
-   }
-   return ret;
+   scribble = kvmalloc_array(cnt, obj_size, flags);
+   if (!scribble)
+   return -ENOMEM;
+
+   kvfree(percpu->scribble);
+
+   percpu->scribble = scribble;
+   percpu->scribble_obj_size = obj_size;
+   return 0;
 }
 
 static int resize_chunks(struct r5conf *conf, int new_disks, int new_sectors)
@@ -2272,23 +2268,18 @@ static int resize_chunks(struct r5conf *conf, int 
new_disks, int new_sectors)
return 0;
mddev_suspend(conf->mddev);
get_online_cpus();
+
for_each_present_cpu(cpu) {
struct raid5_percpu *percpu;
-   struct flex_array *scribble;
 
percpu = per_cpu_ptr(conf->percpu, cpu);
-   scribble = scribble_alloc(new_disks,
- new_sectors / STRIPE_SECTORS,
- GFP_NOIO);
-
-   if (scribble) {
-   flex_array_free(percpu->scribble);
-   percpu->scribble = scribble;
-   } else {
-   err = -ENOMEM;
+   err = scribble_alloc(percpu, new_disks,
+new_sectors / STRIPE_SECTORS,
+GFP_NOIO);
+   if (err)
break;
-   }
}
+
put_online_cpus();
mddev_resume(conf->mddev);