| Message ID | d6f0117f-0de5-86d6-b568-5454df627f50@lightnvm.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On 5 Mar 2018, at 15.16, Matias Bjørling <mb@lightnvm.io> wrote: > > On 03/05/2018 02:45 PM, Javier González wrote: >>> On 5 Mar 2018, at 14.38, Matias Bjørling <mb@lightnvm.io> wrote: >>> >>> On 03/01/2018 08:29 PM, Javier González wrote: >>>>> On 1 Mar 2018, at 19.49, Matias Bjørling <mb@lightnvm.io> wrote: >>>>> >>>>> On 03/01/2018 04:59 PM, Javier González wrote: >>>>>> Refactor init and exit sequences to eliminate dependencies among init >>>>>> modules and improve readability. >>>>>> Signed-off-by: Javier González <javier@cnexlabs.com> >>>>>> --- >>>>>> drivers/lightnvm/pblk-init.c | 415 +++++++++++++++++++++---------------------- >>>>>> 1 file changed, 206 insertions(+), 209 deletions(-) >>>>>> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >>>>>> index 25fc70ca07f7..87c390667dd6 100644 >>>>>> --- a/drivers/lightnvm/pblk-init.c >>>>>> +++ b/drivers/lightnvm/pblk-init.c >>>>>> @@ -103,7 +103,40 @@ static void pblk_l2p_free(struct pblk *pblk) >>>>>> vfree(pblk->trans_map); >>>>>> } >>>>>> -static int pblk_l2p_init(struct pblk *pblk) >>>>>> +static int pblk_l2p_recover(struct pblk *pblk, bool factory_init) >>>>>> +{ >>>>>> + struct pblk_line *line = NULL; >>>>>> + >>>>>> + if (factory_init) { >>>>>> + pblk_setup_uuid(pblk); >>>>>> + } else { >>>>>> + line = pblk_recov_l2p(pblk); >>>>>> + if (IS_ERR(line)) { >>>>>> + pr_err("pblk: could not recover l2p table\n"); >>>>>> + return -EFAULT; >>>>>> + } >>>>>> + } >>>>>> + >>>>>> +#ifdef CONFIG_NVM_DEBUG >>>>>> + pr_info("pblk init: L2P CRC: %x\n", pblk_l2p_crc(pblk)); >>>>>> +#endif >>>>>> + >>>>>> + /* Free full lines directly as GC has not been started yet */ >>>>>> + pblk_gc_free_full_lines(pblk); >>>>>> + >>>>>> + if (!line) { >>>>>> + /* Configure next line for user data */ >>>>>> + line = pblk_line_get_first_data(pblk); >>>>>> + if (!line) { >>>>>> + pr_err("pblk: line list corrupted\n"); >>>>>> + return -EFAULT; >>>>>> + } >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static int pblk_l2p_init(struct pblk *pblk, bool factory_init) >>>>>> { >>>>>> sector_t i; >>>>>> struct ppa_addr ppa; >>>>>> @@ -119,7 +152,7 @@ static int pblk_l2p_init(struct pblk *pblk) >>>>>> for (i = 0; i < pblk->rl.nr_secs; i++) >>>>>> pblk_trans_map_set(pblk, i, ppa); >>>>>> - return 0; >>>>>> + return pblk_l2p_recover(pblk, factory_init); >>>>>> } >>>>>> static void pblk_rwb_free(struct pblk *pblk) >>>>>> @@ -159,7 +192,13 @@ static int pblk_set_ppaf(struct pblk *pblk) >>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>> struct nvm_geo *geo = &dev->geo; >>>>>> struct nvm_addr_format ppaf = geo->ppaf; >>>>>> - int power_len; >>>>>> + int mod, power_len; >>>>>> + >>>>>> + div_u64_rem(geo->sec_per_chk, pblk->min_write_pgs, &mod); >>>>>> + if (mod) { >>>>>> + pr_err("pblk: bad configuration of sectors/pages\n"); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> /* Re-calculate channel and lun format to adapt to configuration */ >>>>>> power_len = get_count_order(geo->nr_chnls); >>>>>> @@ -252,12 +291,39 @@ static int pblk_core_init(struct pblk *pblk) >>>>>> { >>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>> struct nvm_geo *geo = &dev->geo; >>>>>> + int max_write_ppas; >>>>>> + >>>>>> + atomic64_set(&pblk->user_wa, 0); >>>>>> + atomic64_set(&pblk->pad_wa, 0); >>>>>> + atomic64_set(&pblk->gc_wa, 0); >>>>>> + pblk->user_rst_wa = 0; >>>>>> + pblk->pad_rst_wa = 0; >>>>>> + pblk->gc_rst_wa = 0; >>>>>> + >>>>>> + atomic64_set(&pblk->nr_flush, 0); >>>>>> + pblk->nr_flush_rst = 0; >>>>>> pblk->pgs_in_buffer = NVM_MEM_PAGE_WRITE * geo->sec_per_pg * >>>>>> geo->nr_planes * geo->all_luns; >>>>>> + pblk->min_write_pgs = geo->sec_per_pl * (geo->sec_size / PAGE_SIZE); >>>>>> + max_write_ppas = pblk->min_write_pgs * geo->all_luns; >>>>>> + pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA); >>>>>> + pblk_set_sec_per_write(pblk, pblk->min_write_pgs); >>>>>> + >>>>>> + if (pblk->max_write_pgs > PBLK_MAX_REQ_ADDRS) { >>>>>> + pr_err("pblk: vector list too big(%u > %u)\n", >>>>>> + pblk->max_write_pgs, PBLK_MAX_REQ_ADDRS); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + >>>>>> + pblk->pad_dist = kzalloc((pblk->min_write_pgs - 1) * sizeof(atomic64_t), >>>>>> + GFP_KERNEL); >>>>>> + if (!pblk->pad_dist) >>>>>> + return -ENOMEM; >>>>>> + >>>>>> if (pblk_init_global_caches(pblk)) >>>>>> - return -ENOMEM; >>>>>> + goto fail_free_pad_dist; >>>>>> /* Internal bios can be at most the sectors signaled by the device. */ >>>>>> pblk->page_bio_pool = mempool_create_page_pool(NVM_MAX_VLBA, 0); >>>>>> @@ -307,10 +373,8 @@ static int pblk_core_init(struct pblk *pblk) >>>>>> if (pblk_set_ppaf(pblk)) >>>>>> goto free_r_end_wq; >>>>>> - if (pblk_rwb_init(pblk)) >>>>>> - goto free_r_end_wq; >>>>>> - >>>>>> INIT_LIST_HEAD(&pblk->compl_list); >>>>>> + >>>>>> return 0; >>>>>> free_r_end_wq: >>>>>> @@ -333,6 +397,8 @@ static int pblk_core_init(struct pblk *pblk) >>>>>> mempool_destroy(pblk->page_bio_pool); >>>>>> free_global_caches: >>>>>> pblk_free_global_caches(pblk); >>>>>> +fail_free_pad_dist: >>>>>> + kfree(pblk->pad_dist); >>>>>> return -ENOMEM; >>>>>> } >>>>>> @@ -354,14 +420,8 @@ static void pblk_core_free(struct pblk *pblk) >>>>>> mempool_destroy(pblk->e_rq_pool); >>>>>> mempool_destroy(pblk->w_rq_pool); >>>>>> - pblk_rwb_free(pblk); >>>>>> - >>>>>> pblk_free_global_caches(pblk); >>>>>> -} >>>>>> - >>>>>> -static void pblk_luns_free(struct pblk *pblk) >>>>>> -{ >>>>>> - kfree(pblk->luns); >>>>>> + kfree(pblk->pad_dist); >>>>>> } >>>>>> static void pblk_line_mg_free(struct pblk *pblk) >>>>>> @@ -378,8 +438,6 @@ static void pblk_line_mg_free(struct pblk *pblk) >>>>>> pblk_mfree(l_mg->eline_meta[i]->buf, l_mg->emeta_alloc_type); >>>>>> kfree(l_mg->eline_meta[i]); >>>>>> } >>>>>> - >>>>>> - kfree(pblk->lines); >>>>>> } >>>>>> static void pblk_line_meta_free(struct pblk_line *line) >>>>>> @@ -402,6 +460,11 @@ static void pblk_lines_free(struct pblk *pblk) >>>>>> pblk_line_meta_free(line); >>>>>> } >>>>>> spin_unlock(&l_mg->free_lock); >>>>>> + >>>>>> + pblk_line_mg_free(pblk); >>>>>> + >>>>>> + kfree(pblk->luns); >>>>>> + kfree(pblk->lines); >>>>>> } >>>>>> static int pblk_bb_get_tbl(struct nvm_tgt_dev *dev, struct pblk_lun *rlun, >>>>>> @@ -476,7 +539,7 @@ static int pblk_bb_line(struct pblk *pblk, struct pblk_line *line, >>>>>> return bb_cnt; >>>>>> } >>>>>> -static int pblk_luns_init(struct pblk *pblk, struct ppa_addr *luns) >>>>>> +static int pblk_luns_init(struct pblk *pblk) >>>>>> { >>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>> struct nvm_geo *geo = &dev->geo; >>>>>> @@ -501,7 +564,7 @@ static int pblk_luns_init(struct pblk *pblk, struct ppa_addr *luns) >>>>>> int lunid = lun_raw + ch * geo->nr_luns; >>>>>> rlun = &pblk->luns[i]; >>>>>> - rlun->bppa = luns[lunid]; >>>>>> + rlun->bppa = dev->luns[lunid]; >>>>>> sema_init(&rlun->wr_sem, 1); >>>>>> } >>>>>> @@ -509,38 +572,6 @@ static int pblk_luns_init(struct pblk *pblk, struct ppa_addr *luns) >>>>>> return 0; >>>>>> } >>>>>> -static int pblk_lines_configure(struct pblk *pblk, int flags) >>>>>> -{ >>>>>> - struct pblk_line *line = NULL; >>>>>> - int ret = 0; >>>>>> - >>>>>> - if (!(flags & NVM_TARGET_FACTORY)) { >>>>>> - line = pblk_recov_l2p(pblk); >>>>>> - if (IS_ERR(line)) { >>>>>> - pr_err("pblk: could not recover l2p table\n"); >>>>>> - ret = -EFAULT; >>>>>> - } >>>>>> - } >>>>>> - >>>>>> -#ifdef CONFIG_NVM_DEBUG >>>>>> - pr_info("pblk init: L2P CRC: %x\n", pblk_l2p_crc(pblk)); >>>>>> -#endif >>>>>> - >>>>>> - /* Free full lines directly as GC has not been started yet */ >>>>>> - pblk_gc_free_full_lines(pblk); >>>>>> - >>>>>> - if (!line) { >>>>>> - /* Configure next line for user data */ >>>>>> - line = pblk_line_get_first_data(pblk); >>>>>> - if (!line) { >>>>>> - pr_err("pblk: line list corrupted\n"); >>>>>> - ret = -EFAULT; >>>>>> - } >>>>>> - } >>>>>> - >>>>>> - return ret; >>>>>> -} >>>>>> - >>>>>> /* See comment over struct line_emeta definition */ >>>>>> static unsigned int calc_emeta_len(struct pblk *pblk) >>>>>> { >>>>>> @@ -606,11 +637,70 @@ static void pblk_set_provision(struct pblk *pblk, long nr_free_blks) >>>>>> atomic_set(&pblk->rl.free_user_blocks, nr_free_blks); >>>>>> } >>>>>> -static int pblk_lines_alloc_metadata(struct pblk *pblk) >>>>>> +static int pblk_setup_line_meta(struct pblk *pblk, struct pblk_line *line, >>>>>> + void *chunk_log, long *nr_bad_blks) >>>>>> { >>>>>> + struct pblk_line_meta *lm = &pblk->lm; >>>>>> + >>>>>> + line->blk_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>> + if (!line->blk_bitmap) >>>>>> + return -ENOMEM; >>>>>> + >>>>>> + line->erase_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>> + if (!line->erase_bitmap) { >>>>>> + kfree(line->blk_bitmap); >>>>>> + return -ENOMEM; >>>>>> + } >>>>>> + >>>>>> + *nr_bad_blks = pblk_bb_line(pblk, line, chunk_log, lm->blk_per_line); >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static int pblk_line_mg_init(struct pblk *pblk) >>>>>> +{ >>>>>> + struct nvm_tgt_dev *dev = pblk->dev; >>>>>> + struct nvm_geo *geo = &dev->geo; >>>>>> struct pblk_line_mgmt *l_mg = &pblk->l_mg; >>>>>> struct pblk_line_meta *lm = &pblk->lm; >>>>>> - int i; >>>>>> + int i, bb_distance; >>>>>> + >>>>>> + l_mg->nr_lines = geo->nr_chks; >>>>>> + l_mg->log_line = l_mg->data_line = NULL; >>>>>> + l_mg->l_seq_nr = l_mg->d_seq_nr = 0; >>>>>> + l_mg->nr_free_lines = 0; >>>>>> + bitmap_zero(&l_mg->meta_bitmap, PBLK_DATA_LINES); >>>>>> + >>>>>> + INIT_LIST_HEAD(&l_mg->free_list); >>>>>> + INIT_LIST_HEAD(&l_mg->corrupt_list); >>>>>> + INIT_LIST_HEAD(&l_mg->bad_list); >>>>>> + INIT_LIST_HEAD(&l_mg->gc_full_list); >>>>>> + INIT_LIST_HEAD(&l_mg->gc_high_list); >>>>>> + INIT_LIST_HEAD(&l_mg->gc_mid_list); >>>>>> + INIT_LIST_HEAD(&l_mg->gc_low_list); >>>>>> + INIT_LIST_HEAD(&l_mg->gc_empty_list); >>>>>> + >>>>>> + INIT_LIST_HEAD(&l_mg->emeta_list); >>>>>> + >>>>>> + l_mg->gc_lists[0] = &l_mg->gc_high_list; >>>>>> + l_mg->gc_lists[1] = &l_mg->gc_mid_list; >>>>>> + l_mg->gc_lists[2] = &l_mg->gc_low_list; >>>>>> + >>>>>> + spin_lock_init(&l_mg->free_lock); >>>>>> + spin_lock_init(&l_mg->close_lock); >>>>>> + spin_lock_init(&l_mg->gc_lock); >>>>>> + >>>>>> + l_mg->vsc_list = kcalloc(l_mg->nr_lines, sizeof(__le32), GFP_KERNEL); >>>>>> + if (!l_mg->vsc_list) >>>>>> + goto fail; >>>>>> + >>>>>> + l_mg->bb_template = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>> + if (!l_mg->bb_template) >>>>>> + goto fail_free_vsc_list; >>>>>> + >>>>>> + l_mg->bb_aux = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>> + if (!l_mg->bb_aux) >>>>>> + goto fail_free_bb_template; >>>>>> /* smeta is always small enough to fit on a kmalloc memory allocation, >>>>>> * emeta depends on the number of LUNs allocated to the pblk instance >>>>>> @@ -656,13 +746,13 @@ static int pblk_lines_alloc_metadata(struct pblk *pblk) >>>>>> } >>>>>> } >>>>>> - l_mg->vsc_list = kcalloc(l_mg->nr_lines, sizeof(__le32), GFP_KERNEL); >>>>>> - if (!l_mg->vsc_list) >>>>>> - goto fail_free_emeta; >>>>>> - >>>>>> for (i = 0; i < l_mg->nr_lines; i++) >>>>>> l_mg->vsc_list[i] = cpu_to_le32(EMPTY_ENTRY); >>>>>> + bb_distance = (geo->all_luns) * geo->ws_opt; >>>>>> + for (i = 0; i < lm->sec_per_line; i += bb_distance) >>>>>> + bitmap_set(l_mg->bb_template, i, geo->ws_opt); >>>>>> + >>>>>> return 0; >>>>>> fail_free_emeta: >>>>>> @@ -673,69 +763,25 @@ static int pblk_lines_alloc_metadata(struct pblk *pblk) >>>>>> kfree(l_mg->eline_meta[i]->buf); >>>>>> kfree(l_mg->eline_meta[i]); >>>>>> } >>>>>> - >>>>>> fail_free_smeta: >>>>>> for (i = 0; i < PBLK_DATA_LINES; i++) >>>>>> kfree(l_mg->sline_meta[i]); >>>>>> - >>>>>> + kfree(l_mg->bb_aux); >>>>>> +fail_free_bb_template: >>>>>> + kfree(l_mg->bb_template); >>>>>> +fail_free_vsc_list: >>>>>> + kfree(l_mg->vsc_list); >>>>>> +fail: >>>>>> return -ENOMEM; >>>>>> } >>>>>> -static int pblk_setup_line_meta(struct pblk *pblk, struct pblk_line *line, >>>>>> - void *chunk_log, long *nr_bad_blks) >>>>>> -{ >>>>>> - struct pblk_line_meta *lm = &pblk->lm; >>>>>> - >>>>>> - line->blk_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>> - if (!line->blk_bitmap) >>>>>> - return -ENOMEM; >>>>>> - >>>>>> - line->erase_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>> - if (!line->erase_bitmap) { >>>>>> - kfree(line->blk_bitmap); >>>>>> - return -ENOMEM; >>>>>> - } >>>>>> - >>>>>> - *nr_bad_blks = pblk_bb_line(pblk, line, chunk_log, lm->blk_per_line); >>>>>> - >>>>>> - return 0; >>>>>> -} >>>>>> - >>>>>> -static int pblk_lines_init(struct pblk *pblk) >>>>>> +static int pblk_line_meta_init(struct pblk *pblk) >>>>>> { >>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>> struct nvm_geo *geo = &dev->geo; >>>>>> - struct pblk_line_mgmt *l_mg = &pblk->l_mg; >>>>>> struct pblk_line_meta *lm = &pblk->lm; >>>>>> - struct pblk_line *line; >>>>>> - void *chunk_log; >>>>>> unsigned int smeta_len, emeta_len; >>>>>> - long nr_bad_blks = 0, nr_free_blks = 0; >>>>>> - int bb_distance, max_write_ppas, mod; >>>>>> - int i, ret; >>>>>> - >>>>>> - pblk->min_write_pgs = geo->sec_per_pl * (geo->sec_size / PAGE_SIZE); >>>>>> - max_write_ppas = pblk->min_write_pgs * geo->all_luns; >>>>>> - pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA); >>>>>> - pblk_set_sec_per_write(pblk, pblk->min_write_pgs); >>>>>> - >>>>>> - if (pblk->max_write_pgs > PBLK_MAX_REQ_ADDRS) { >>>>>> - pr_err("pblk: vector list too big(%u > %u)\n", >>>>>> - pblk->max_write_pgs, PBLK_MAX_REQ_ADDRS); >>>>>> - return -EINVAL; >>>>>> - } >>>>>> - >>>>>> - div_u64_rem(geo->sec_per_chk, pblk->min_write_pgs, &mod); >>>>>> - if (mod) { >>>>>> - pr_err("pblk: bad configuration of sectors/pages\n"); >>>>>> - return -EINVAL; >>>>>> - } >>>>>> - >>>>>> - l_mg->nr_lines = geo->nr_chks; >>>>>> - l_mg->log_line = l_mg->data_line = NULL; >>>>>> - l_mg->l_seq_nr = l_mg->d_seq_nr = 0; >>>>>> - l_mg->nr_free_lines = 0; >>>>>> - bitmap_zero(&l_mg->meta_bitmap, PBLK_DATA_LINES); >>>>>> + int i; >>>>>> lm->sec_per_line = geo->sec_per_chk * geo->all_luns; >>>>>> lm->blk_per_line = geo->all_luns; >>>>>> @@ -787,58 +833,43 @@ static int pblk_lines_init(struct pblk *pblk) >>>>>> return -EINVAL; >>>>>> } >>>>>> - ret = pblk_lines_alloc_metadata(pblk); >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static int pblk_lines_init(struct pblk *pblk) >>>>>> +{ >>>>>> + struct pblk_line_mgmt *l_mg = &pblk->l_mg; >>>>>> + struct pblk_line_meta *lm = &pblk->lm; >>>>>> + struct pblk_line *line; >>>>>> + void *chunk_log; >>>>>> + long nr_bad_blks = 0, nr_free_blks = 0; >>>>>> + int i, ret; >>>>>> + >>>>>> + ret = pblk_line_meta_init(pblk); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + >>>>>> + ret = pblk_line_mg_init(pblk); >>>>>> if (ret) >>>>>> return ret; >>>>>> - l_mg->bb_template = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>> - if (!l_mg->bb_template) { >>>>>> - ret = -ENOMEM; >>>>>> + ret = pblk_luns_init(pblk); >>>>>> + if (ret) >>>>>> goto fail_free_meta; >>>>>> - } >>>>>> - >>>>>> - l_mg->bb_aux = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>> - if (!l_mg->bb_aux) { >>>>>> - ret = -ENOMEM; >>>>>> - goto fail_free_bb_template; >>>>>> - } >>>>>> - >>>>>> - bb_distance = (geo->all_luns) * geo->sec_per_pl; >>>>>> - for (i = 0; i < lm->sec_per_line; i += bb_distance) >>>>>> - bitmap_set(l_mg->bb_template, i, geo->sec_per_pl); >>>>>> - >>>>>> - INIT_LIST_HEAD(&l_mg->free_list); >>>>>> - INIT_LIST_HEAD(&l_mg->corrupt_list); >>>>>> - INIT_LIST_HEAD(&l_mg->bad_list); >>>>>> - INIT_LIST_HEAD(&l_mg->gc_full_list); >>>>>> - INIT_LIST_HEAD(&l_mg->gc_high_list); >>>>>> - INIT_LIST_HEAD(&l_mg->gc_mid_list); >>>>>> - INIT_LIST_HEAD(&l_mg->gc_low_list); >>>>>> - INIT_LIST_HEAD(&l_mg->gc_empty_list); >>>>>> - >>>>>> - INIT_LIST_HEAD(&l_mg->emeta_list); >>>>>> - >>>>>> - l_mg->gc_lists[0] = &l_mg->gc_high_list; >>>>>> - l_mg->gc_lists[1] = &l_mg->gc_mid_list; >>>>>> - l_mg->gc_lists[2] = &l_mg->gc_low_list; >>>>>> - >>>>>> - spin_lock_init(&l_mg->free_lock); >>>>>> - spin_lock_init(&l_mg->close_lock); >>>>>> - spin_lock_init(&l_mg->gc_lock); >>>>>> - >>>>>> - pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), >>>>>> - GFP_KERNEL); >>>>>> - if (!pblk->lines) { >>>>>> - ret = -ENOMEM; >>>>>> - goto fail_free_bb_aux; >>>>>> - } >>>>>> chunk_log = pblk_bb_get_log(pblk); >>>>>> if (IS_ERR(chunk_log)) { >>>>>> pr_err("pblk: could not get bad block log (%lu)\n", >>>>>> PTR_ERR(chunk_log)); >>>>>> ret = PTR_ERR(chunk_log); >>>>>> - goto fail_free_lines; >>>>>> + goto fail_free_luns; >>>>>> + } >>>>>> + >>>>>> + pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), >>>>>> + GFP_KERNEL); >>>>>> + if (!pblk->lines) { >>>>>> + ret = -ENOMEM; >>>>>> + goto fail_free_chunk_log; >>>>>> } >>>>>> for (i = 0; i < l_mg->nr_lines; i++) { >>>>>> @@ -856,7 +887,7 @@ static int pblk_lines_init(struct pblk *pblk) >>>>>> ret = pblk_setup_line_meta(pblk, line, chunk_log, &nr_bad_blks); >>>>>> if (ret) >>>>>> - goto fail_free_chunk_log; >>>>>> + goto fail_free_lines; >>>>>> chk_in_line = lm->blk_per_line - nr_bad_blks; >>>>>> if (nr_bad_blks < 0 || nr_bad_blks > lm->blk_per_line || >>>>>> @@ -878,16 +909,14 @@ static int pblk_lines_init(struct pblk *pblk) >>>>>> kfree(chunk_log); >>>>>> return 0; >>>>>> -fail_free_chunk_log: >>>>>> - kfree(chunk_log); >>>>>> +fail_free_lines: >>>>>> while (--i >= 0) >>>>>> pblk_line_meta_free(&pblk->lines[i]); >>>>>> -fail_free_lines: >>>>>> kfree(pblk->lines); >>>>>> -fail_free_bb_aux: >>>>>> - kfree(l_mg->bb_aux); >>>>>> -fail_free_bb_template: >>>>>> - kfree(l_mg->bb_template); >>>>>> +fail_free_chunk_log: >>>>>> + kfree(chunk_log); >>>>>> +fail_free_luns: >>>>>> + kfree(pblk->luns); >>>>>> fail_free_meta: >>>>>> pblk_line_mg_free(pblk); >>>>>> @@ -930,12 +959,10 @@ static void pblk_writer_stop(struct pblk *pblk) >>>>>> static void pblk_free(struct pblk *pblk) >>>>>> { >>>>>> - pblk_luns_free(pblk); >>>>>> pblk_lines_free(pblk); >>>>>> - kfree(pblk->pad_dist); >>>>>> - pblk_line_mg_free(pblk); >>>>>> - pblk_core_free(pblk); >>>>>> pblk_l2p_free(pblk); >>>>>> + pblk_rwb_free(pblk); >>>>>> + pblk_core_free(pblk); >>>>>> kfree(pblk); >>>>>> } >>>>>> @@ -1000,19 +1027,6 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>>>>> spin_lock_init(&pblk->trans_lock); >>>>>> spin_lock_init(&pblk->lock); >>>>>> - if (flags & NVM_TARGET_FACTORY) >>>>>> - pblk_setup_uuid(pblk); >>>>>> - >>>>>> - atomic64_set(&pblk->user_wa, 0); >>>>>> - atomic64_set(&pblk->pad_wa, 0); >>>>>> - atomic64_set(&pblk->gc_wa, 0); >>>>>> - pblk->user_rst_wa = 0; >>>>>> - pblk->pad_rst_wa = 0; >>>>>> - pblk->gc_rst_wa = 0; >>>>>> - >>>>>> - atomic64_set(&pblk->nr_flush, 0); >>>>>> - pblk->nr_flush_rst = 0; >>>>>> - >>>>>> #ifdef CONFIG_NVM_DEBUG >>>>>> atomic_long_set(&pblk->inflight_writes, 0); >>>>>> atomic_long_set(&pblk->padded_writes, 0); >>>>>> @@ -1036,48 +1050,35 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>>>>> atomic_long_set(&pblk->write_failed, 0); >>>>>> atomic_long_set(&pblk->erase_failed, 0); >>>>>> - ret = pblk_luns_init(pblk, dev->luns); >>>>>> - if (ret) { >>>>>> - pr_err("pblk: could not initialize luns\n"); >>>>>> - goto fail; >>>>>> - } >>>>>> - >>>>>> - ret = pblk_lines_init(pblk); >>>>>> - if (ret) { >>>>>> - pr_err("pblk: could not initialize lines\n"); >>>>>> - goto fail_free_luns; >>>>>> - } >>>>>> - >>>>>> - pblk->pad_dist = kzalloc((pblk->min_write_pgs - 1) * sizeof(atomic64_t), >>>>>> - GFP_KERNEL); >>>>>> - if (!pblk->pad_dist) { >>>>>> - ret = -ENOMEM; >>>>>> - goto fail_free_line_meta; >>>>>> - } >>>>>> - >>>>>> ret = pblk_core_init(pblk); >>>>>> if (ret) { >>>>>> pr_err("pblk: could not initialize core\n"); >>>>>> - goto fail_free_pad_dist; >>>>>> + goto fail; >>>>>> } >>>>>> - ret = pblk_l2p_init(pblk); >>>>>> + ret = pblk_lines_init(pblk); >>>>>> if (ret) { >>>>>> - pr_err("pblk: could not initialize maps\n"); >>>>>> + pr_err("pblk: could not initialize lines\n"); >>>>>> goto fail_free_core; >>>>>> } >>>>>> - ret = pblk_lines_configure(pblk, flags); >>>>>> + ret = pblk_rwb_init(pblk); >>>>>> if (ret) { >>>>>> - pr_err("pblk: could not configure lines\n"); >>>>>> - goto fail_free_l2p; >>>>>> + pr_err("pblk: could not initialize write buffer\n"); >>>>>> + goto fail_free_lines; >>>>>> + } >>>>>> + >>>>>> + ret = pblk_l2p_init(pblk, flags & NVM_TARGET_FACTORY); >>>>>> + if (ret) { >>>>>> + pr_err("pblk: could not initialize maps\n"); >>>>>> + goto fail_free_rwb; >>>>>> } >>>>>> ret = pblk_writer_init(pblk); >>>>>> if (ret) { >>>>>> if (ret != -EINTR) >>>>>> pr_err("pblk: could not initialize write thread\n"); >>>>>> - goto fail_free_lines; >>>>>> + goto fail_free_l2p; >>>>>> } >>>>>> ret = pblk_gc_init(pblk); >>>>>> @@ -1112,18 +1113,14 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>>>>> fail_stop_writer: >>>>>> pblk_writer_stop(pblk); >>>>>> -fail_free_lines: >>>>>> - pblk_lines_free(pblk); >>>>>> fail_free_l2p: >>>>>> pblk_l2p_free(pblk); >>>>>> +fail_free_rwb: >>>>>> + pblk_rwb_free(pblk); >>>>>> +fail_free_lines: >>>>>> + pblk_lines_free(pblk); >>>>>> fail_free_core: >>>>>> pblk_core_free(pblk); >>>>>> -fail_free_pad_dist: >>>>>> - kfree(pblk->pad_dist); >>>>>> -fail_free_line_meta: >>>>>> - pblk_line_mg_free(pblk); >>>>>> -fail_free_luns: >>>>>> - pblk_luns_free(pblk); >>>>>> fail: >>>>>> kfree(pblk); >>>>>> return ERR_PTR(ret); >>>>> >>>>> Thanks. I'm not able to squash it without conflict. Is it based on for-4.17/core? >>>> Yes, it's based on for-4.17/core, but I can see a small conflict due to >>>> the patches applied in the middle of these two. How do you want to do >>>> it? We can keep them separately, or do a rebase on the current patches. >>> >>> This is all a bit of a mess- Can you send a fix I can apply that fixes >>> up the 0-day, and then this larger patch can be applied afterwards? >>> (e.g., before at after your other patches) >> Not really. The problem is that the original refactoring for the bad >> block table was a bandage on the init/exit sequence problem. The >> init/ext patch is the one that does it properly. >> What we can do, since this is only in your tree, is rebase and pull the >> bad block patch up and then merge with init/exit. This way we fix the >> original problem and the introduced double free is non-existing. >> I can do the rebasing this evening and put it in a different branch you >> can review. > > No thanks. I fixed it up with this: > > diff --git i/drivers/lightnvm/pblk-init.c w/drivers/lightnvm/pblk-init.c > index a2b54a88968b..c31e24aabc18 100644 > --- i/drivers/lightnvm/pblk-init.c > +++ w/drivers/lightnvm/pblk-init.c > @@ -838,7 +838,7 @@ static int pblk_lines_init(struct pblk *pblk) > pr_err("pblk: could not get bad block log (%lu)\n", > PTR_ERR(chunk_log)); > ret = PTR_ERR(chunk_log); > - goto fail_free_lines; > + goto fail_free_bb_aux; > } > > for (i = 0; i < l_mg->nr_lines; i++) { > @@ -882,8 +882,6 @@ static int pblk_lines_init(struct pblk *pblk) > kfree(chunk_log); > while (--i >= 0) > pblk_line_meta_free(&pblk->lines[i]); > -fail_free_lines: > - kfree(pblk->lines); > fail_free_bb_aux: > kfree(l_mg->bb_aux); > fail_free_bb_template: It's not that simple... There are other double frees due to the way l_mg is allocated, which is fixed on the other patch. Javier
On 03/05/2018 03:18 PM, Javier González wrote: >> On 5 Mar 2018, at 15.16, Matias Bjørling <mb@lightnvm.io> wrote: >> >> On 03/05/2018 02:45 PM, Javier González wrote: >>>> On 5 Mar 2018, at 14.38, Matias Bjørling <mb@lightnvm.io> wrote: >>>> >>>> On 03/01/2018 08:29 PM, Javier González wrote: >>>>>> On 1 Mar 2018, at 19.49, Matias Bjørling <mb@lightnvm.io> wrote: >>>>>> >>>>>> On 03/01/2018 04:59 PM, Javier González wrote: >>>>>>> Refactor init and exit sequences to eliminate dependencies among init >>>>>>> modules and improve readability. >>>>>>> Signed-off-by: Javier González <javier@cnexlabs.com> >>>>>>> --- >>>>>>> drivers/lightnvm/pblk-init.c | 415 +++++++++++++++++++++---------------------- >>>>>>> 1 file changed, 206 insertions(+), 209 deletions(-) >>>>>>> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >>>>>>> index 25fc70ca07f7..87c390667dd6 100644 >>>>>>> --- a/drivers/lightnvm/pblk-init.c >>>>>>> +++ b/drivers/lightnvm/pblk-init.c >>>>>>> @@ -103,7 +103,40 @@ static void pblk_l2p_free(struct pblk *pblk) >>>>>>> vfree(pblk->trans_map); >>>>>>> } >>>>>>> -static int pblk_l2p_init(struct pblk *pblk) >>>>>>> +static int pblk_l2p_recover(struct pblk *pblk, bool factory_init) >>>>>>> +{ >>>>>>> + struct pblk_line *line = NULL; >>>>>>> + >>>>>>> + if (factory_init) { >>>>>>> + pblk_setup_uuid(pblk); >>>>>>> + } else { >>>>>>> + line = pblk_recov_l2p(pblk); >>>>>>> + if (IS_ERR(line)) { >>>>>>> + pr_err("pblk: could not recover l2p table\n"); >>>>>>> + return -EFAULT; >>>>>>> + } >>>>>>> + } >>>>>>> + >>>>>>> +#ifdef CONFIG_NVM_DEBUG >>>>>>> + pr_info("pblk init: L2P CRC: %x\n", pblk_l2p_crc(pblk)); >>>>>>> +#endif >>>>>>> + >>>>>>> + /* Free full lines directly as GC has not been started yet */ >>>>>>> + pblk_gc_free_full_lines(pblk); >>>>>>> + >>>>>>> + if (!line) { >>>>>>> + /* Configure next line for user data */ >>>>>>> + line = pblk_line_get_first_data(pblk); >>>>>>> + if (!line) { >>>>>>> + pr_err("pblk: line list corrupted\n"); >>>>>>> + return -EFAULT; >>>>>>> + } >>>>>>> + } >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static int pblk_l2p_init(struct pblk *pblk, bool factory_init) >>>>>>> { >>>>>>> sector_t i; >>>>>>> struct ppa_addr ppa; >>>>>>> @@ -119,7 +152,7 @@ static int pblk_l2p_init(struct pblk *pblk) >>>>>>> for (i = 0; i < pblk->rl.nr_secs; i++) >>>>>>> pblk_trans_map_set(pblk, i, ppa); >>>>>>> - return 0; >>>>>>> + return pblk_l2p_recover(pblk, factory_init); >>>>>>> } >>>>>>> static void pblk_rwb_free(struct pblk *pblk) >>>>>>> @@ -159,7 +192,13 @@ static int pblk_set_ppaf(struct pblk *pblk) >>>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>>> struct nvm_geo *geo = &dev->geo; >>>>>>> struct nvm_addr_format ppaf = geo->ppaf; >>>>>>> - int power_len; >>>>>>> + int mod, power_len; >>>>>>> + >>>>>>> + div_u64_rem(geo->sec_per_chk, pblk->min_write_pgs, &mod); >>>>>>> + if (mod) { >>>>>>> + pr_err("pblk: bad configuration of sectors/pages\n"); >>>>>>> + return -EINVAL; >>>>>>> + } >>>>>>> /* Re-calculate channel and lun format to adapt to configuration */ >>>>>>> power_len = get_count_order(geo->nr_chnls); >>>>>>> @@ -252,12 +291,39 @@ static int pblk_core_init(struct pblk *pblk) >>>>>>> { >>>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>>> struct nvm_geo *geo = &dev->geo; >>>>>>> + int max_write_ppas; >>>>>>> + >>>>>>> + atomic64_set(&pblk->user_wa, 0); >>>>>>> + atomic64_set(&pblk->pad_wa, 0); >>>>>>> + atomic64_set(&pblk->gc_wa, 0); >>>>>>> + pblk->user_rst_wa = 0; >>>>>>> + pblk->pad_rst_wa = 0; >>>>>>> + pblk->gc_rst_wa = 0; >>>>>>> + >>>>>>> + atomic64_set(&pblk->nr_flush, 0); >>>>>>> + pblk->nr_flush_rst = 0; >>>>>>> pblk->pgs_in_buffer = NVM_MEM_PAGE_WRITE * geo->sec_per_pg * >>>>>>> geo->nr_planes * geo->all_luns; >>>>>>> + pblk->min_write_pgs = geo->sec_per_pl * (geo->sec_size / PAGE_SIZE); >>>>>>> + max_write_ppas = pblk->min_write_pgs * geo->all_luns; >>>>>>> + pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA); >>>>>>> + pblk_set_sec_per_write(pblk, pblk->min_write_pgs); >>>>>>> + >>>>>>> + if (pblk->max_write_pgs > PBLK_MAX_REQ_ADDRS) { >>>>>>> + pr_err("pblk: vector list too big(%u > %u)\n", >>>>>>> + pblk->max_write_pgs, PBLK_MAX_REQ_ADDRS); >>>>>>> + return -EINVAL; >>>>>>> + } >>>>>>> + >>>>>>> + pblk->pad_dist = kzalloc((pblk->min_write_pgs - 1) * sizeof(atomic64_t), >>>>>>> + GFP_KERNEL); >>>>>>> + if (!pblk->pad_dist) >>>>>>> + return -ENOMEM; >>>>>>> + >>>>>>> if (pblk_init_global_caches(pblk)) >>>>>>> - return -ENOMEM; >>>>>>> + goto fail_free_pad_dist; >>>>>>> /* Internal bios can be at most the sectors signaled by the device. */ >>>>>>> pblk->page_bio_pool = mempool_create_page_pool(NVM_MAX_VLBA, 0); >>>>>>> @@ -307,10 +373,8 @@ static int pblk_core_init(struct pblk *pblk) >>>>>>> if (pblk_set_ppaf(pblk)) >>>>>>> goto free_r_end_wq; >>>>>>> - if (pblk_rwb_init(pblk)) >>>>>>> - goto free_r_end_wq; >>>>>>> - >>>>>>> INIT_LIST_HEAD(&pblk->compl_list); >>>>>>> + >>>>>>> return 0; >>>>>>> free_r_end_wq: >>>>>>> @@ -333,6 +397,8 @@ static int pblk_core_init(struct pblk *pblk) >>>>>>> mempool_destroy(pblk->page_bio_pool); >>>>>>> free_global_caches: >>>>>>> pblk_free_global_caches(pblk); >>>>>>> +fail_free_pad_dist: >>>>>>> + kfree(pblk->pad_dist); >>>>>>> return -ENOMEM; >>>>>>> } >>>>>>> @@ -354,14 +420,8 @@ static void pblk_core_free(struct pblk *pblk) >>>>>>> mempool_destroy(pblk->e_rq_pool); >>>>>>> mempool_destroy(pblk->w_rq_pool); >>>>>>> - pblk_rwb_free(pblk); >>>>>>> - >>>>>>> pblk_free_global_caches(pblk); >>>>>>> -} >>>>>>> - >>>>>>> -static void pblk_luns_free(struct pblk *pblk) >>>>>>> -{ >>>>>>> - kfree(pblk->luns); >>>>>>> + kfree(pblk->pad_dist); >>>>>>> } >>>>>>> static void pblk_line_mg_free(struct pblk *pblk) >>>>>>> @@ -378,8 +438,6 @@ static void pblk_line_mg_free(struct pblk *pblk) >>>>>>> pblk_mfree(l_mg->eline_meta[i]->buf, l_mg->emeta_alloc_type); >>>>>>> kfree(l_mg->eline_meta[i]); >>>>>>> } >>>>>>> - >>>>>>> - kfree(pblk->lines); >>>>>>> } >>>>>>> static void pblk_line_meta_free(struct pblk_line *line) >>>>>>> @@ -402,6 +460,11 @@ static void pblk_lines_free(struct pblk *pblk) >>>>>>> pblk_line_meta_free(line); >>>>>>> } >>>>>>> spin_unlock(&l_mg->free_lock); >>>>>>> + >>>>>>> + pblk_line_mg_free(pblk); >>>>>>> + >>>>>>> + kfree(pblk->luns); >>>>>>> + kfree(pblk->lines); >>>>>>> } >>>>>>> static int pblk_bb_get_tbl(struct nvm_tgt_dev *dev, struct pblk_lun *rlun, >>>>>>> @@ -476,7 +539,7 @@ static int pblk_bb_line(struct pblk *pblk, struct pblk_line *line, >>>>>>> return bb_cnt; >>>>>>> } >>>>>>> -static int pblk_luns_init(struct pblk *pblk, struct ppa_addr *luns) >>>>>>> +static int pblk_luns_init(struct pblk *pblk) >>>>>>> { >>>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>>> struct nvm_geo *geo = &dev->geo; >>>>>>> @@ -501,7 +564,7 @@ static int pblk_luns_init(struct pblk *pblk, struct ppa_addr *luns) >>>>>>> int lunid = lun_raw + ch * geo->nr_luns; >>>>>>> rlun = &pblk->luns[i]; >>>>>>> - rlun->bppa = luns[lunid]; >>>>>>> + rlun->bppa = dev->luns[lunid]; >>>>>>> sema_init(&rlun->wr_sem, 1); >>>>>>> } >>>>>>> @@ -509,38 +572,6 @@ static int pblk_luns_init(struct pblk *pblk, struct ppa_addr *luns) >>>>>>> return 0; >>>>>>> } >>>>>>> -static int pblk_lines_configure(struct pblk *pblk, int flags) >>>>>>> -{ >>>>>>> - struct pblk_line *line = NULL; >>>>>>> - int ret = 0; >>>>>>> - >>>>>>> - if (!(flags & NVM_TARGET_FACTORY)) { >>>>>>> - line = pblk_recov_l2p(pblk); >>>>>>> - if (IS_ERR(line)) { >>>>>>> - pr_err("pblk: could not recover l2p table\n"); >>>>>>> - ret = -EFAULT; >>>>>>> - } >>>>>>> - } >>>>>>> - >>>>>>> -#ifdef CONFIG_NVM_DEBUG >>>>>>> - pr_info("pblk init: L2P CRC: %x\n", pblk_l2p_crc(pblk)); >>>>>>> -#endif >>>>>>> - >>>>>>> - /* Free full lines directly as GC has not been started yet */ >>>>>>> - pblk_gc_free_full_lines(pblk); >>>>>>> - >>>>>>> - if (!line) { >>>>>>> - /* Configure next line for user data */ >>>>>>> - line = pblk_line_get_first_data(pblk); >>>>>>> - if (!line) { >>>>>>> - pr_err("pblk: line list corrupted\n"); >>>>>>> - ret = -EFAULT; >>>>>>> - } >>>>>>> - } >>>>>>> - >>>>>>> - return ret; >>>>>>> -} >>>>>>> - >>>>>>> /* See comment over struct line_emeta definition */ >>>>>>> static unsigned int calc_emeta_len(struct pblk *pblk) >>>>>>> { >>>>>>> @@ -606,11 +637,70 @@ static void pblk_set_provision(struct pblk *pblk, long nr_free_blks) >>>>>>> atomic_set(&pblk->rl.free_user_blocks, nr_free_blks); >>>>>>> } >>>>>>> -static int pblk_lines_alloc_metadata(struct pblk *pblk) >>>>>>> +static int pblk_setup_line_meta(struct pblk *pblk, struct pblk_line *line, >>>>>>> + void *chunk_log, long *nr_bad_blks) >>>>>>> { >>>>>>> + struct pblk_line_meta *lm = &pblk->lm; >>>>>>> + >>>>>>> + line->blk_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>>> + if (!line->blk_bitmap) >>>>>>> + return -ENOMEM; >>>>>>> + >>>>>>> + line->erase_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>>> + if (!line->erase_bitmap) { >>>>>>> + kfree(line->blk_bitmap); >>>>>>> + return -ENOMEM; >>>>>>> + } >>>>>>> + >>>>>>> + *nr_bad_blks = pblk_bb_line(pblk, line, chunk_log, lm->blk_per_line); >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static int pblk_line_mg_init(struct pblk *pblk) >>>>>>> +{ >>>>>>> + struct nvm_tgt_dev *dev = pblk->dev; >>>>>>> + struct nvm_geo *geo = &dev->geo; >>>>>>> struct pblk_line_mgmt *l_mg = &pblk->l_mg; >>>>>>> struct pblk_line_meta *lm = &pblk->lm; >>>>>>> - int i; >>>>>>> + int i, bb_distance; >>>>>>> + >>>>>>> + l_mg->nr_lines = geo->nr_chks; >>>>>>> + l_mg->log_line = l_mg->data_line = NULL; >>>>>>> + l_mg->l_seq_nr = l_mg->d_seq_nr = 0; >>>>>>> + l_mg->nr_free_lines = 0; >>>>>>> + bitmap_zero(&l_mg->meta_bitmap, PBLK_DATA_LINES); >>>>>>> + >>>>>>> + INIT_LIST_HEAD(&l_mg->free_list); >>>>>>> + INIT_LIST_HEAD(&l_mg->corrupt_list); >>>>>>> + INIT_LIST_HEAD(&l_mg->bad_list); >>>>>>> + INIT_LIST_HEAD(&l_mg->gc_full_list); >>>>>>> + INIT_LIST_HEAD(&l_mg->gc_high_list); >>>>>>> + INIT_LIST_HEAD(&l_mg->gc_mid_list); >>>>>>> + INIT_LIST_HEAD(&l_mg->gc_low_list); >>>>>>> + INIT_LIST_HEAD(&l_mg->gc_empty_list); >>>>>>> + >>>>>>> + INIT_LIST_HEAD(&l_mg->emeta_list); >>>>>>> + >>>>>>> + l_mg->gc_lists[0] = &l_mg->gc_high_list; >>>>>>> + l_mg->gc_lists[1] = &l_mg->gc_mid_list; >>>>>>> + l_mg->gc_lists[2] = &l_mg->gc_low_list; >>>>>>> + >>>>>>> + spin_lock_init(&l_mg->free_lock); >>>>>>> + spin_lock_init(&l_mg->close_lock); >>>>>>> + spin_lock_init(&l_mg->gc_lock); >>>>>>> + >>>>>>> + l_mg->vsc_list = kcalloc(l_mg->nr_lines, sizeof(__le32), GFP_KERNEL); >>>>>>> + if (!l_mg->vsc_list) >>>>>>> + goto fail; >>>>>>> + >>>>>>> + l_mg->bb_template = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>>> + if (!l_mg->bb_template) >>>>>>> + goto fail_free_vsc_list; >>>>>>> + >>>>>>> + l_mg->bb_aux = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>>> + if (!l_mg->bb_aux) >>>>>>> + goto fail_free_bb_template; >>>>>>> /* smeta is always small enough to fit on a kmalloc memory allocation, >>>>>>> * emeta depends on the number of LUNs allocated to the pblk instance >>>>>>> @@ -656,13 +746,13 @@ static int pblk_lines_alloc_metadata(struct pblk *pblk) >>>>>>> } >>>>>>> } >>>>>>> - l_mg->vsc_list = kcalloc(l_mg->nr_lines, sizeof(__le32), GFP_KERNEL); >>>>>>> - if (!l_mg->vsc_list) >>>>>>> - goto fail_free_emeta; >>>>>>> - >>>>>>> for (i = 0; i < l_mg->nr_lines; i++) >>>>>>> l_mg->vsc_list[i] = cpu_to_le32(EMPTY_ENTRY); >>>>>>> + bb_distance = (geo->all_luns) * geo->ws_opt; >>>>>>> + for (i = 0; i < lm->sec_per_line; i += bb_distance) >>>>>>> + bitmap_set(l_mg->bb_template, i, geo->ws_opt); >>>>>>> + >>>>>>> return 0; >>>>>>> fail_free_emeta: >>>>>>> @@ -673,69 +763,25 @@ static int pblk_lines_alloc_metadata(struct pblk *pblk) >>>>>>> kfree(l_mg->eline_meta[i]->buf); >>>>>>> kfree(l_mg->eline_meta[i]); >>>>>>> } >>>>>>> - >>>>>>> fail_free_smeta: >>>>>>> for (i = 0; i < PBLK_DATA_LINES; i++) >>>>>>> kfree(l_mg->sline_meta[i]); >>>>>>> - >>>>>>> + kfree(l_mg->bb_aux); >>>>>>> +fail_free_bb_template: >>>>>>> + kfree(l_mg->bb_template); >>>>>>> +fail_free_vsc_list: >>>>>>> + kfree(l_mg->vsc_list); >>>>>>> +fail: >>>>>>> return -ENOMEM; >>>>>>> } >>>>>>> -static int pblk_setup_line_meta(struct pblk *pblk, struct pblk_line *line, >>>>>>> - void *chunk_log, long *nr_bad_blks) >>>>>>> -{ >>>>>>> - struct pblk_line_meta *lm = &pblk->lm; >>>>>>> - >>>>>>> - line->blk_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>>> - if (!line->blk_bitmap) >>>>>>> - return -ENOMEM; >>>>>>> - >>>>>>> - line->erase_bitmap = kzalloc(lm->blk_bitmap_len, GFP_KERNEL); >>>>>>> - if (!line->erase_bitmap) { >>>>>>> - kfree(line->blk_bitmap); >>>>>>> - return -ENOMEM; >>>>>>> - } >>>>>>> - >>>>>>> - *nr_bad_blks = pblk_bb_line(pblk, line, chunk_log, lm->blk_per_line); >>>>>>> - >>>>>>> - return 0; >>>>>>> -} >>>>>>> - >>>>>>> -static int pblk_lines_init(struct pblk *pblk) >>>>>>> +static int pblk_line_meta_init(struct pblk *pblk) >>>>>>> { >>>>>>> struct nvm_tgt_dev *dev = pblk->dev; >>>>>>> struct nvm_geo *geo = &dev->geo; >>>>>>> - struct pblk_line_mgmt *l_mg = &pblk->l_mg; >>>>>>> struct pblk_line_meta *lm = &pblk->lm; >>>>>>> - struct pblk_line *line; >>>>>>> - void *chunk_log; >>>>>>> unsigned int smeta_len, emeta_len; >>>>>>> - long nr_bad_blks = 0, nr_free_blks = 0; >>>>>>> - int bb_distance, max_write_ppas, mod; >>>>>>> - int i, ret; >>>>>>> - >>>>>>> - pblk->min_write_pgs = geo->sec_per_pl * (geo->sec_size / PAGE_SIZE); >>>>>>> - max_write_ppas = pblk->min_write_pgs * geo->all_luns; >>>>>>> - pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA); >>>>>>> - pblk_set_sec_per_write(pblk, pblk->min_write_pgs); >>>>>>> - >>>>>>> - if (pblk->max_write_pgs > PBLK_MAX_REQ_ADDRS) { >>>>>>> - pr_err("pblk: vector list too big(%u > %u)\n", >>>>>>> - pblk->max_write_pgs, PBLK_MAX_REQ_ADDRS); >>>>>>> - return -EINVAL; >>>>>>> - } >>>>>>> - >>>>>>> - div_u64_rem(geo->sec_per_chk, pblk->min_write_pgs, &mod); >>>>>>> - if (mod) { >>>>>>> - pr_err("pblk: bad configuration of sectors/pages\n"); >>>>>>> - return -EINVAL; >>>>>>> - } >>>>>>> - >>>>>>> - l_mg->nr_lines = geo->nr_chks; >>>>>>> - l_mg->log_line = l_mg->data_line = NULL; >>>>>>> - l_mg->l_seq_nr = l_mg->d_seq_nr = 0; >>>>>>> - l_mg->nr_free_lines = 0; >>>>>>> - bitmap_zero(&l_mg->meta_bitmap, PBLK_DATA_LINES); >>>>>>> + int i; >>>>>>> lm->sec_per_line = geo->sec_per_chk * geo->all_luns; >>>>>>> lm->blk_per_line = geo->all_luns; >>>>>>> @@ -787,58 +833,43 @@ static int pblk_lines_init(struct pblk *pblk) >>>>>>> return -EINVAL; >>>>>>> } >>>>>>> - ret = pblk_lines_alloc_metadata(pblk); >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static int pblk_lines_init(struct pblk *pblk) >>>>>>> +{ >>>>>>> + struct pblk_line_mgmt *l_mg = &pblk->l_mg; >>>>>>> + struct pblk_line_meta *lm = &pblk->lm; >>>>>>> + struct pblk_line *line; >>>>>>> + void *chunk_log; >>>>>>> + long nr_bad_blks = 0, nr_free_blks = 0; >>>>>>> + int i, ret; >>>>>>> + >>>>>>> + ret = pblk_line_meta_init(pblk); >>>>>>> + if (ret) >>>>>>> + return ret; >>>>>>> + >>>>>>> + ret = pblk_line_mg_init(pblk); >>>>>>> if (ret) >>>>>>> return ret; >>>>>>> - l_mg->bb_template = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>>> - if (!l_mg->bb_template) { >>>>>>> - ret = -ENOMEM; >>>>>>> + ret = pblk_luns_init(pblk); >>>>>>> + if (ret) >>>>>>> goto fail_free_meta; >>>>>>> - } >>>>>>> - >>>>>>> - l_mg->bb_aux = kzalloc(lm->sec_bitmap_len, GFP_KERNEL); >>>>>>> - if (!l_mg->bb_aux) { >>>>>>> - ret = -ENOMEM; >>>>>>> - goto fail_free_bb_template; >>>>>>> - } >>>>>>> - >>>>>>> - bb_distance = (geo->all_luns) * geo->sec_per_pl; >>>>>>> - for (i = 0; i < lm->sec_per_line; i += bb_distance) >>>>>>> - bitmap_set(l_mg->bb_template, i, geo->sec_per_pl); >>>>>>> - >>>>>>> - INIT_LIST_HEAD(&l_mg->free_list); >>>>>>> - INIT_LIST_HEAD(&l_mg->corrupt_list); >>>>>>> - INIT_LIST_HEAD(&l_mg->bad_list); >>>>>>> - INIT_LIST_HEAD(&l_mg->gc_full_list); >>>>>>> - INIT_LIST_HEAD(&l_mg->gc_high_list); >>>>>>> - INIT_LIST_HEAD(&l_mg->gc_mid_list); >>>>>>> - INIT_LIST_HEAD(&l_mg->gc_low_list); >>>>>>> - INIT_LIST_HEAD(&l_mg->gc_empty_list); >>>>>>> - >>>>>>> - INIT_LIST_HEAD(&l_mg->emeta_list); >>>>>>> - >>>>>>> - l_mg->gc_lists[0] = &l_mg->gc_high_list; >>>>>>> - l_mg->gc_lists[1] = &l_mg->gc_mid_list; >>>>>>> - l_mg->gc_lists[2] = &l_mg->gc_low_list; >>>>>>> - >>>>>>> - spin_lock_init(&l_mg->free_lock); >>>>>>> - spin_lock_init(&l_mg->close_lock); >>>>>>> - spin_lock_init(&l_mg->gc_lock); >>>>>>> - >>>>>>> - pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), >>>>>>> - GFP_KERNEL); >>>>>>> - if (!pblk->lines) { >>>>>>> - ret = -ENOMEM; >>>>>>> - goto fail_free_bb_aux; >>>>>>> - } >>>>>>> chunk_log = pblk_bb_get_log(pblk); >>>>>>> if (IS_ERR(chunk_log)) { >>>>>>> pr_err("pblk: could not get bad block log (%lu)\n", >>>>>>> PTR_ERR(chunk_log)); >>>>>>> ret = PTR_ERR(chunk_log); >>>>>>> - goto fail_free_lines; >>>>>>> + goto fail_free_luns; >>>>>>> + } >>>>>>> + >>>>>>> + pblk->lines = kcalloc(l_mg->nr_lines, sizeof(struct pblk_line), >>>>>>> + GFP_KERNEL); >>>>>>> + if (!pblk->lines) { >>>>>>> + ret = -ENOMEM; >>>>>>> + goto fail_free_chunk_log; >>>>>>> } >>>>>>> for (i = 0; i < l_mg->nr_lines; i++) { >>>>>>> @@ -856,7 +887,7 @@ static int pblk_lines_init(struct pblk *pblk) >>>>>>> ret = pblk_setup_line_meta(pblk, line, chunk_log, &nr_bad_blks); >>>>>>> if (ret) >>>>>>> - goto fail_free_chunk_log; >>>>>>> + goto fail_free_lines; >>>>>>> chk_in_line = lm->blk_per_line - nr_bad_blks; >>>>>>> if (nr_bad_blks < 0 || nr_bad_blks > lm->blk_per_line || >>>>>>> @@ -878,16 +909,14 @@ static int pblk_lines_init(struct pblk *pblk) >>>>>>> kfree(chunk_log); >>>>>>> return 0; >>>>>>> -fail_free_chunk_log: >>>>>>> - kfree(chunk_log); >>>>>>> +fail_free_lines: >>>>>>> while (--i >= 0) >>>>>>> pblk_line_meta_free(&pblk->lines[i]); >>>>>>> -fail_free_lines: >>>>>>> kfree(pblk->lines); >>>>>>> -fail_free_bb_aux: >>>>>>> - kfree(l_mg->bb_aux); >>>>>>> -fail_free_bb_template: >>>>>>> - kfree(l_mg->bb_template); >>>>>>> +fail_free_chunk_log: >>>>>>> + kfree(chunk_log); >>>>>>> +fail_free_luns: >>>>>>> + kfree(pblk->luns); >>>>>>> fail_free_meta: >>>>>>> pblk_line_mg_free(pblk); >>>>>>> @@ -930,12 +959,10 @@ static void pblk_writer_stop(struct pblk *pblk) >>>>>>> static void pblk_free(struct pblk *pblk) >>>>>>> { >>>>>>> - pblk_luns_free(pblk); >>>>>>> pblk_lines_free(pblk); >>>>>>> - kfree(pblk->pad_dist); >>>>>>> - pblk_line_mg_free(pblk); >>>>>>> - pblk_core_free(pblk); >>>>>>> pblk_l2p_free(pblk); >>>>>>> + pblk_rwb_free(pblk); >>>>>>> + pblk_core_free(pblk); >>>>>>> kfree(pblk); >>>>>>> } >>>>>>> @@ -1000,19 +1027,6 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>>>>>> spin_lock_init(&pblk->trans_lock); >>>>>>> spin_lock_init(&pblk->lock); >>>>>>> - if (flags & NVM_TARGET_FACTORY) >>>>>>> - pblk_setup_uuid(pblk); >>>>>>> - >>>>>>> - atomic64_set(&pblk->user_wa, 0); >>>>>>> - atomic64_set(&pblk->pad_wa, 0); >>>>>>> - atomic64_set(&pblk->gc_wa, 0); >>>>>>> - pblk->user_rst_wa = 0; >>>>>>> - pblk->pad_rst_wa = 0; >>>>>>> - pblk->gc_rst_wa = 0; >>>>>>> - >>>>>>> - atomic64_set(&pblk->nr_flush, 0); >>>>>>> - pblk->nr_flush_rst = 0; >>>>>>> - >>>>>>> #ifdef CONFIG_NVM_DEBUG >>>>>>> atomic_long_set(&pblk->inflight_writes, 0); >>>>>>> atomic_long_set(&pblk->padded_writes, 0); >>>>>>> @@ -1036,48 +1050,35 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>>>>>> atomic_long_set(&pblk->write_failed, 0); >>>>>>> atomic_long_set(&pblk->erase_failed, 0); >>>>>>> - ret = pblk_luns_init(pblk, dev->luns); >>>>>>> - if (ret) { >>>>>>> - pr_err("pblk: could not initialize luns\n"); >>>>>>> - goto fail; >>>>>>> - } >>>>>>> - >>>>>>> - ret = pblk_lines_init(pblk); >>>>>>> - if (ret) { >>>>>>> - pr_err("pblk: could not initialize lines\n"); >>>>>>> - goto fail_free_luns; >>>>>>> - } >>>>>>> - >>>>>>> - pblk->pad_dist = kzalloc((pblk->min_write_pgs - 1) * sizeof(atomic64_t), >>>>>>> - GFP_KERNEL); >>>>>>> - if (!pblk->pad_dist) { >>>>>>> - ret = -ENOMEM; >>>>>>> - goto fail_free_line_meta; >>>>>>> - } >>>>>>> - >>>>>>> ret = pblk_core_init(pblk); >>>>>>> if (ret) { >>>>>>> pr_err("pblk: could not initialize core\n"); >>>>>>> - goto fail_free_pad_dist; >>>>>>> + goto fail; >>>>>>> } >>>>>>> - ret = pblk_l2p_init(pblk); >>>>>>> + ret = pblk_lines_init(pblk); >>>>>>> if (ret) { >>>>>>> - pr_err("pblk: could not initialize maps\n"); >>>>>>> + pr_err("pblk: could not initialize lines\n"); >>>>>>> goto fail_free_core; >>>>>>> } >>>>>>> - ret = pblk_lines_configure(pblk, flags); >>>>>>> + ret = pblk_rwb_init(pblk); >>>>>>> if (ret) { >>>>>>> - pr_err("pblk: could not configure lines\n"); >>>>>>> - goto fail_free_l2p; >>>>>>> + pr_err("pblk: could not initialize write buffer\n"); >>>>>>> + goto fail_free_lines; >>>>>>> + } >>>>>>> + >>>>>>> + ret = pblk_l2p_init(pblk, flags & NVM_TARGET_FACTORY); >>>>>>> + if (ret) { >>>>>>> + pr_err("pblk: could not initialize maps\n"); >>>>>>> + goto fail_free_rwb; >>>>>>> } >>>>>>> ret = pblk_writer_init(pblk); >>>>>>> if (ret) { >>>>>>> if (ret != -EINTR) >>>>>>> pr_err("pblk: could not initialize write thread\n"); >>>>>>> - goto fail_free_lines; >>>>>>> + goto fail_free_l2p; >>>>>>> } >>>>>>> ret = pblk_gc_init(pblk); >>>>>>> @@ -1112,18 +1113,14 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>>>>>> fail_stop_writer: >>>>>>> pblk_writer_stop(pblk); >>>>>>> -fail_free_lines: >>>>>>> - pblk_lines_free(pblk); >>>>>>> fail_free_l2p: >>>>>>> pblk_l2p_free(pblk); >>>>>>> +fail_free_rwb: >>>>>>> + pblk_rwb_free(pblk); >>>>>>> +fail_free_lines: >>>>>>> + pblk_lines_free(pblk); >>>>>>> fail_free_core: >>>>>>> pblk_core_free(pblk); >>>>>>> -fail_free_pad_dist: >>>>>>> - kfree(pblk->pad_dist); >>>>>>> -fail_free_line_meta: >>>>>>> - pblk_line_mg_free(pblk); >>>>>>> -fail_free_luns: >>>>>>> - pblk_luns_free(pblk); >>>>>>> fail: >>>>>>> kfree(pblk); >>>>>>> return ERR_PTR(ret); >>>>>> >>>>>> Thanks. I'm not able to squash it without conflict. Is it based on for-4.17/core? >>>>> Yes, it's based on for-4.17/core, but I can see a small conflict due to >>>>> the patches applied in the middle of these two. How do you want to do >>>>> it? We can keep them separately, or do a rebase on the current patches. >>>> >>>> This is all a bit of a mess- Can you send a fix I can apply that fixes >>>> up the 0-day, and then this larger patch can be applied afterwards? >>>> (e.g., before at after your other patches) >>> Not really. The problem is that the original refactoring for the bad >>> block table was a bandage on the init/exit sequence problem. The >>> init/ext patch is the one that does it properly. >>> What we can do, since this is only in your tree, is rebase and pull the >>> bad block patch up and then merge with init/exit. This way we fix the >>> original problem and the introduced double free is non-existing. >>> I can do the rebasing this evening and put it in a different branch you >>> can review. >> >> No thanks. I fixed it up with this: >> >> diff --git i/drivers/lightnvm/pblk-init.c w/drivers/lightnvm/pblk-init.c >> index a2b54a88968b..c31e24aabc18 100644 >> --- i/drivers/lightnvm/pblk-init.c >> +++ w/drivers/lightnvm/pblk-init.c >> @@ -838,7 +838,7 @@ static int pblk_lines_init(struct pblk *pblk) >> pr_err("pblk: could not get bad block log (%lu)\n", >> PTR_ERR(chunk_log)); >> ret = PTR_ERR(chunk_log); >> - goto fail_free_lines; >> + goto fail_free_bb_aux; >> } >> >> for (i = 0; i < l_mg->nr_lines; i++) { >> @@ -882,8 +882,6 @@ static int pblk_lines_init(struct pblk *pblk) >> kfree(chunk_log); >> while (--i >= 0) >> pblk_line_meta_free(&pblk->lines[i]); >> -fail_free_lines: >> - kfree(pblk->lines); >> fail_free_bb_aux: >> kfree(l_mg->bb_aux); >> fail_free_bb_template: > > It's not that simple... There are other double frees due to the way l_mg > is allocated, which is fixed on the other patch. > I know that. This was to fix up the original patch. I do not want to rebase ack'ed patches.
diff --git i/drivers/lightnvm/pblk-init.c w/drivers/lightnvm/pblk-init.c index a2b54a88968b..c31e24aabc18 100644 --- i/drivers/lightnvm/pblk-init.c +++ w/drivers/lightnvm/pblk-init.c @@ -838,7 +838,7 @@ static int pblk_lines_init(struct pblk *pblk) pr_err("pblk: could not get bad block log (%lu)\n", PTR_ERR(chunk_log)); ret = PTR_ERR(chunk_log); - goto fail_free_lines; + goto fail_free_bb_aux; }