From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932157AbeCEOQV (ORCPT ); Mon, 5 Mar 2018 09:16:21 -0500 Received: from mail-lf0-f68.google.com ([209.85.215.68]:38751 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932112AbeCEOQR (ORCPT ); Mon, 5 Mar 2018 09:16:17 -0500 X-Google-Smtp-Source: AG47ELvOdp+q9mOJujyqz/7GSsdRkfjkecRv5iHcFblCy/13LaPk7xTv5pj0xN9i8vsW1zJDfSl7EA== Subject: Re: [PATCH] lightnvm: pblk: refactor init/exit sequences To: =?UTF-8?Q?Javier_Gonz=c3=a1lez?= Cc: linux-block@vger.kernel.org, LKML , Dan Carpenter References: <1519919998-20829-1-git-send-email-javier@cnexlabs.com> <1519919998-20829-2-git-send-email-javier@cnexlabs.com> <8586a9e4-c76f-e38a-edcf-201cc7e2ddfb@lightnvm.io> <64fdacf9-fc89-48fb-4a8d-a8dada708329@lightnvm.io> <12DCD7A9-01CB-4CCE-8F6A-6B892056F5F0@javigon.com> From: =?UTF-8?Q?Matias_Bj=c3=b8rling?= Message-ID: Date: Mon, 5 Mar 2018 15:16:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <12DCD7A9-01CB-4CCE-8F6A-6B892056F5F0@javigon.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/05/2018 02:45 PM, Javier González wrote: >> On 5 Mar 2018, at 14.38, Matias Bjørling wrote: >> >> On 03/01/2018 08:29 PM, Javier González wrote: >>>> On 1 Mar 2018, at 19.49, Matias Bjørling 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 >>>>> --- >>>>> 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: