| Message ID | 1523874066-4459-1-git-send-email-javier@cnexlabs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 4/16/18 12:21 PM, Javier González wrote: > Allocate line bitmaps outside of the line lock on line preparation. > > Signed-off-by: Javier González <javier@cnexlabs.com> The patch description tells what the patch does, it should tell why the change the done. > --- > drivers/lightnvm/pblk-core.c | 96 +++++++++++++++++++++++++------------------- > 1 file changed, 55 insertions(+), 41 deletions(-) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index 5f960a6609c8..7762e89984ee 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -1058,6 +1058,25 @@ static int pblk_line_init_metadata(struct pblk *pblk, struct pblk_line *line, > return 1; > } > > +static int pblk_line_alloc_bitmaps(struct pblk *pblk, struct pblk_line *line) > +{ > + struct pblk_line_meta *lm = &pblk->lm; > + > + line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); > + if (!line->map_bitmap) > + return -ENOMEM; > + > + /* will be initialized using bb info from map_bitmap */ > + line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_KERNEL); > + if (!line->invalid_bitmap) { > + kfree(line->map_bitmap); > + line->map_bitmap = NULL; > + return -ENOMEM; > + } > + > + return 0; > +} > + > /* For now lines are always assumed full lines. Thus, smeta former and current > * lun bitmaps are omitted. > */ > @@ -1162,18 +1181,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) > { > struct pblk_line_meta *lm = &pblk->lm; > int blk_in_line = atomic_read(&line->blk_in_line); > - int blk_to_erase, ret; > - > - line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_ATOMIC); > - if (!line->map_bitmap) > - return -ENOMEM; > - > - /* will be initialized using bb info from map_bitmap */ > - line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_ATOMIC); > - if (!line->invalid_bitmap) { > - ret = -ENOMEM; > - goto fail_free_map_bitmap; > - } > + int blk_to_erase; > > /* Bad blocks do not need to be erased */ > bitmap_copy(line->erase_bitmap, line->blk_bitmap, lm->blk_per_line); > @@ -1191,15 +1199,15 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) > } > > if (blk_in_line < lm->min_blk_line) { > - ret = -EAGAIN; > - goto fail_free_invalid_bitmap; > + spin_unlock(&line->lock); > + return -EAGAIN; > } > > if (line->state != PBLK_LINESTATE_FREE) { > WARN(1, "pblk: corrupted line %d, state %d\n", > line->id, line->state); > - ret = -EINTR; > - goto fail_free_invalid_bitmap; > + spin_unlock(&line->lock); > + return -EINTR; > } > > line->state = PBLK_LINESTATE_OPEN; > @@ -1213,16 +1221,6 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) > kref_init(&line->ref); > > return 0; > - > -fail_free_invalid_bitmap: > - spin_unlock(&line->lock); > - kfree(line->invalid_bitmap); > - line->invalid_bitmap = NULL; > -fail_free_map_bitmap: > - kfree(line->map_bitmap); > - line->map_bitmap = NULL; > - > - return ret; > } > > int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line) > @@ -1242,13 +1240,15 @@ int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line) > } > spin_unlock(&l_mg->free_lock); > > - pblk_rl_free_lines_dec(&pblk->rl, line, true); > + if (pblk_line_alloc_bitmaps(pblk, line)) > + return -EINTR; Why return -EINTR, instead of the return value from (0, -ENOMEM) from pblk_line_alloc_bitmap? > > if (!pblk_line_init_bb(pblk, line, 0)) { > list_add(&line->list, &l_mg->free_list); > return -EINTR; > } > > + pblk_rl_free_lines_dec(&pblk->rl, line, true); > return 0; > } > > @@ -1260,6 +1260,24 @@ void pblk_line_recov_close(struct pblk *pblk, struct pblk_line *line) > line->emeta = NULL; > } > > +static void pblk_line_clear(struct pblk_line *line) > +{ > + *line->vsc = cpu_to_le32(EMPTY_ENTRY); > + > + line->map_bitmap = NULL; > + line->invalid_bitmap = NULL; > + line->smeta = NULL; > + line->emeta = NULL; > +} Instead of pblk_line_clear, how about pblk_line_reinit?
> On 17 Apr 2018, at 04.48, Matias Bjørling <mb@lightnvm.io> wrote: > > On 4/16/18 12:21 PM, Javier González wrote: >> Allocate line bitmaps outside of the line lock on line preparation. >> Signed-off-by: Javier González <javier@cnexlabs.com> > > > The patch description tells what the patch does, it should tell why > the change the done. > Taking an allocation out of a critical zone should be a reason on itself. >> --- >> drivers/lightnvm/pblk-core.c | 96 +++++++++++++++++++++++++------------------- >> 1 file changed, 55 insertions(+), 41 deletions(-) >> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c >> index 5f960a6609c8..7762e89984ee 100644 >> --- a/drivers/lightnvm/pblk-core.c >> +++ b/drivers/lightnvm/pblk-core.c >> @@ -1058,6 +1058,25 @@ static int pblk_line_init_metadata(struct pblk *pblk, struct pblk_line *line, >> return 1; >> } >> +static int pblk_line_alloc_bitmaps(struct pblk *pblk, struct pblk_line *line) >> +{ >> + struct pblk_line_meta *lm = &pblk->lm; >> + >> + line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >> + if (!line->map_bitmap) >> + return -ENOMEM; >> + >> + /* will be initialized using bb info from map_bitmap */ >> + line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_KERNEL); >> + if (!line->invalid_bitmap) { >> + kfree(line->map_bitmap); >> + line->map_bitmap = NULL; >> + return -ENOMEM; >> + } >> + >> + return 0; >> +} >> + >> /* For now lines are always assumed full lines. Thus, smeta former and current >> * lun bitmaps are omitted. >> */ >> @@ -1162,18 +1181,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) >> { >> struct pblk_line_meta *lm = &pblk->lm; >> int blk_in_line = atomic_read(&line->blk_in_line); >> - int blk_to_erase, ret; >> - >> - line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_ATOMIC); >> - if (!line->map_bitmap) >> - return -ENOMEM; >> - >> - /* will be initialized using bb info from map_bitmap */ >> - line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_ATOMIC); >> - if (!line->invalid_bitmap) { >> - ret = -ENOMEM; >> - goto fail_free_map_bitmap; >> - } >> + int blk_to_erase; >> /* Bad blocks do not need to be erased */ >> bitmap_copy(line->erase_bitmap, line->blk_bitmap, lm->blk_per_line); >> @@ -1191,15 +1199,15 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) >> } >> if (blk_in_line < lm->min_blk_line) { >> - ret = -EAGAIN; >> - goto fail_free_invalid_bitmap; >> + spin_unlock(&line->lock); >> + return -EAGAIN; >> } >> if (line->state != PBLK_LINESTATE_FREE) { >> WARN(1, "pblk: corrupted line %d, state %d\n", >> line->id, line->state); >> - ret = -EINTR; >> - goto fail_free_invalid_bitmap; >> + spin_unlock(&line->lock); >> + return -EINTR; >> } >> line->state = PBLK_LINESTATE_OPEN; >> @@ -1213,16 +1221,6 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) >> kref_init(&line->ref); >> return 0; >> - >> -fail_free_invalid_bitmap: >> - spin_unlock(&line->lock); >> - kfree(line->invalid_bitmap); >> - line->invalid_bitmap = NULL; >> -fail_free_map_bitmap: >> - kfree(line->map_bitmap); >> - line->map_bitmap = NULL; >> - >> - return ret; >> } >> int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line) >> @@ -1242,13 +1240,15 @@ int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line) >> } >> spin_unlock(&l_mg->free_lock); >> - pblk_rl_free_lines_dec(&pblk->rl, line, true); >> + if (pblk_line_alloc_bitmaps(pblk, line)) >> + return -EINTR; > > Why return -EINTR, instead of the return value from (0, -ENOMEM) from pblk_line_alloc_bitmap? Sure. > > >> if (!pblk_line_init_bb(pblk, line, 0)) { >> list_add(&line->list, &l_mg->free_list); >> return -EINTR; >> } >> + pblk_rl_free_lines_dec(&pblk->rl, line, true); >> return 0; >> } >> @@ -1260,6 +1260,24 @@ void pblk_line_recov_close(struct pblk *pblk, struct pblk_line *line) >> line->emeta = NULL; >> } >> +static void pblk_line_clear(struct pblk_line *line) >> +{ >> + *line->vsc = cpu_to_le32(EMPTY_ENTRY); >> + >> + line->map_bitmap = NULL; >> + line->invalid_bitmap = NULL; >> + line->smeta = NULL; >> + line->emeta = NULL; >> +} > > Instead of pblk_line_clear, how about pblk_line_reinit? Emmmmm... yes, why not. I'll resend. Javier
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c index 5f960a6609c8..7762e89984ee 100644 --- a/drivers/lightnvm/pblk-core.c +++ b/drivers/lightnvm/pblk-core.c @@ -1058,6 +1058,25 @@ static int pblk_line_init_metadata(struct pblk *pblk, struct pblk_line *line, return 1; } +static int pblk_line_alloc_bitmaps(struct pblk *pblk, struct pblk_line *line) +{ + struct pblk_line_meta *lm = &pblk->lm; + + line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); + if (!line->map_bitmap) + return -ENOMEM; + + /* will be initialized using bb info from map_bitmap */ + line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_KERNEL); + if (!line->invalid_bitmap) { + kfree(line->map_bitmap); + line->map_bitmap = NULL; + return -ENOMEM; + } + + return 0; +} + /* For now lines are always assumed full lines. Thus, smeta former and current * lun bitmaps are omitted. */ @@ -1162,18 +1181,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) { struct pblk_line_meta *lm = &pblk->lm; int blk_in_line = atomic_read(&line->blk_in_line); - int blk_to_erase, ret; - - line->map_bitmap = kzalloc(lm->sec_bitmap_len, GFP_ATOMIC); - if (!line->map_bitmap) - return -ENOMEM; - - /* will be initialized using bb info from map_bitmap */ - line->invalid_bitmap = kmalloc(lm->sec_bitmap_len, GFP_ATOMIC); - if (!line->invalid_bitmap) { - ret = -ENOMEM; - goto fail_free_map_bitmap; - } + int blk_to_erase; /* Bad blocks do not need to be erased */ bitmap_copy(line->erase_bitmap, line->blk_bitmap, lm->blk_per_line); @@ -1191,15 +1199,15 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) } if (blk_in_line < lm->min_blk_line) { - ret = -EAGAIN; - goto fail_free_invalid_bitmap; + spin_unlock(&line->lock); + return -EAGAIN; } if (line->state != PBLK_LINESTATE_FREE) { WARN(1, "pblk: corrupted line %d, state %d\n", line->id, line->state); - ret = -EINTR; - goto fail_free_invalid_bitmap; + spin_unlock(&line->lock); + return -EINTR; } line->state = PBLK_LINESTATE_OPEN; @@ -1213,16 +1221,6 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line) kref_init(&line->ref); return 0; - -fail_free_invalid_bitmap: - spin_unlock(&line->lock); - kfree(line->invalid_bitmap); - line->invalid_bitmap = NULL; -fail_free_map_bitmap: - kfree(line->map_bitmap); - line->map_bitmap = NULL; - - return ret; } int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line) @@ -1242,13 +1240,15 @@ int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line) } spin_unlock(&l_mg->free_lock); - pblk_rl_free_lines_dec(&pblk->rl, line, true); + if (pblk_line_alloc_bitmaps(pblk, line)) + return -EINTR; if (!pblk_line_init_bb(pblk, line, 0)) { list_add(&line->list, &l_mg->free_list); return -EINTR; } + pblk_rl_free_lines_dec(&pblk->rl, line, true); return 0; } @@ -1260,6 +1260,24 @@ void pblk_line_recov_close(struct pblk *pblk, struct pblk_line *line) line->emeta = NULL; } +static void pblk_line_clear(struct pblk_line *line) +{ + *line->vsc = cpu_to_le32(EMPTY_ENTRY); + + line->map_bitmap = NULL; + line->invalid_bitmap = NULL; + line->smeta = NULL; + line->emeta = NULL; +} + +void pblk_line_free(struct pblk_line *line) +{ + kfree(line->map_bitmap); + kfree(line->invalid_bitmap); + + pblk_line_clear(line); +} + struct pblk_line *pblk_line_get(struct pblk *pblk) { struct pblk_line_mgmt *l_mg = &pblk->l_mg; @@ -1326,11 +1344,14 @@ static struct pblk_line *pblk_line_retry(struct pblk *pblk, return NULL; } + retry_line->map_bitmap = line->map_bitmap; + retry_line->invalid_bitmap = line->invalid_bitmap; retry_line->smeta = line->smeta; retry_line->emeta = line->emeta; retry_line->meta_line = line->meta_line; - pblk_line_free(line); + pblk_line_clear(line); + l_mg->data_line = retry_line; spin_unlock(&l_mg->free_lock); @@ -1383,6 +1404,9 @@ struct pblk_line *pblk_line_get_first_data(struct pblk *pblk) } spin_unlock(&l_mg->free_lock); + if (pblk_line_alloc_bitmaps(pblk, line)) + return NULL; + if (pblk_line_erase(pblk, line)) { line = pblk_line_retry(pblk, line); if (!line) @@ -1527,6 +1551,9 @@ struct pblk_line *pblk_line_replace_data(struct pblk *pblk) goto retry_erase; } + if (pblk_line_alloc_bitmaps(pblk, new)) + return NULL; + retry_setup: if (!pblk_line_init_metadata(pblk, new, cur)) { new = pblk_line_retry(pblk, new); @@ -1566,19 +1593,6 @@ struct pblk_line *pblk_line_replace_data(struct pblk *pblk) return new; } -void pblk_line_free(struct pblk_line *line) -{ - kfree(line->map_bitmap); - kfree(line->invalid_bitmap); - - *line->vsc = cpu_to_le32(EMPTY_ENTRY); - - line->map_bitmap = NULL; - line->invalid_bitmap = NULL; - line->smeta = NULL; - line->emeta = NULL; -} - static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line) { struct pblk_line_mgmt *l_mg = &pblk->l_mg;
Allocate line bitmaps outside of the line lock on line preparation. Signed-off-by: Javier González <javier@cnexlabs.com> --- drivers/lightnvm/pblk-core.c | 96 +++++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 41 deletions(-)