Re: [PATCH 2/6] md: convert to kvmalloc
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
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
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
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
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
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);