* [V3 PATCH 0/2] lightnvm: pblk: take write semaphore on metadata
@ 2018-08-17 10:19 Javier González
2018-08-17 10:19 ` [PATCH 1/2] lightnvm: pblk: refactor metadata paths Javier González
2018-08-17 10:19 ` [PATCH 2/2] lightnvm: pblk: take write semaphore on metadata Javier González
0 siblings, 2 replies; 5+ messages in thread
From: Javier González @ 2018-08-17 10:19 UTC (permalink / raw)
To: mb; +Cc: axboe, linux-block, linux-kernel, Javier González
# Changes since V2:
- Split the original patch between the metadata refactoring and the
semaphore logic. This simplifies the write path, where the semaphore
is taken.
# Changes singe V1:
- Fix double I/O on the read path (by Matias)
- Improve commit message (by Jens)
This patchset refactors the metadata separate write and read paths,
which simplifies how the semaphore is taken for writes.
Thanks,
Javier
Javier González (2):
lightnvm: pblk: refactor metadata paths
lightnvm: pblk: take write semaphore on metadata
drivers/lightnvm/pblk-core.c | 352 +++++++++++++++++++--------------------
drivers/lightnvm/pblk-gc.c | 2 +-
drivers/lightnvm/pblk-recovery.c | 4 +-
drivers/lightnvm/pblk.h | 5 +-
4 files changed, 178 insertions(+), 185 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] lightnvm: pblk: refactor metadata paths 2018-08-17 10:19 [V3 PATCH 0/2] lightnvm: pblk: take write semaphore on metadata Javier González @ 2018-08-17 10:19 ` Javier González 2018-08-21 11:53 ` Matias Bjørling 2018-08-17 10:19 ` [PATCH 2/2] lightnvm: pblk: take write semaphore on metadata Javier González 1 sibling, 1 reply; 5+ messages in thread From: Javier González @ 2018-08-17 10:19 UTC (permalink / raw) To: mb; +Cc: axboe, linux-block, linux-kernel, Javier González pblk maintains two different metadata paths for smeta and emeta, which store metadata at the start of the line and at the end of the line, respectively. Until now, these path has been common for writing and retrieving metadata, however, as these paths diverge, the common code becomes less clear and unnecessary complicated. In preparation for further changes to the metadata write path, this patch separates the write and read paths for smeta and emeta and removes the synchronous emeta path as it not used anymore (emeta is scheduled asynchronously to prevent jittering due to internal I/Os). Signed-off-by: Javier González <javier@cnexlabs.com> --- drivers/lightnvm/pblk-core.c | 338 ++++++++++++++++++--------------------- drivers/lightnvm/pblk-gc.c | 2 +- drivers/lightnvm/pblk-recovery.c | 4 +- drivers/lightnvm/pblk.h | 5 +- 4 files changed, 164 insertions(+), 185 deletions(-) diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c index 72de7456845b..52306573cc0e 100644 --- a/drivers/lightnvm/pblk-core.c +++ b/drivers/lightnvm/pblk-core.c @@ -621,12 +621,137 @@ u64 pblk_lookup_page(struct pblk *pblk, struct pblk_line *line) return paddr; } -/* - * Submit emeta to one LUN in the raid line at the time to avoid a deadlock when - * taking the per LUN semaphore. - */ -static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, - void *emeta_buf, u64 paddr, int dir) +u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line) +{ + struct nvm_tgt_dev *dev = pblk->dev; + struct nvm_geo *geo = &dev->geo; + struct pblk_line_meta *lm = &pblk->lm; + int bit; + + /* This usually only happens on bad lines */ + bit = find_first_zero_bit(line->blk_bitmap, lm->blk_per_line); + if (bit >= lm->blk_per_line) + return -1; + + return bit * geo->ws_opt; +} + +int pblk_line_smeta_read(struct pblk *pblk, struct pblk_line *line) +{ + struct nvm_tgt_dev *dev = pblk->dev; + struct pblk_line_meta *lm = &pblk->lm; + struct bio *bio; + struct nvm_rq rqd; + u64 paddr = pblk_line_smeta_start(pblk, line); + int i, ret; + + memset(&rqd, 0, sizeof(struct nvm_rq)); + + rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, + &rqd.dma_meta_list); + if (!rqd.meta_list) + return -ENOMEM; + + rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; + rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; + + bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); + if (IS_ERR(bio)) { + ret = PTR_ERR(bio); + goto free_ppa_list; + } + + bio->bi_iter.bi_sector = 0; /* internal bio */ + bio_set_op_attrs(bio, REQ_OP_READ, 0); + + rqd.bio = bio; + rqd.opcode = NVM_OP_PREAD; + rqd.nr_ppas = lm->smeta_sec; + rqd.is_seq = 1; + + for (i = 0; i < lm->smeta_sec; i++, paddr++) + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); + + ret = pblk_submit_io_sync(pblk, &rqd); + if (ret) { + pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); + bio_put(bio); + goto free_ppa_list; + } + + atomic_dec(&pblk->inflight_io); + + if (rqd.error) + pblk_log_read_err(pblk, &rqd); + +free_ppa_list: + nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); + return ret; +} + +static int pblk_line_smeta_write(struct pblk *pblk, struct pblk_line *line, + u64 paddr) +{ + struct nvm_tgt_dev *dev = pblk->dev; + struct pblk_line_meta *lm = &pblk->lm; + struct bio *bio; + struct nvm_rq rqd; + __le64 *lba_list = emeta_to_lbas(pblk, line->emeta->buf); + __le64 addr_empty = cpu_to_le64(ADDR_EMPTY); + int i, ret; + + memset(&rqd, 0, sizeof(struct nvm_rq)); + + rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, + &rqd.dma_meta_list); + if (!rqd.meta_list) + return -ENOMEM; + + rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; + rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; + + bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); + if (IS_ERR(bio)) { + ret = PTR_ERR(bio); + goto free_ppa_list; + } + + bio->bi_iter.bi_sector = 0; /* internal bio */ + bio_set_op_attrs(bio, REQ_OP_WRITE, 0); + + rqd.bio = bio; + rqd.opcode = NVM_OP_PWRITE; + rqd.nr_ppas = lm->smeta_sec; + rqd.is_seq = 1; + + for (i = 0; i < lm->smeta_sec; i++, paddr++) { + struct pblk_sec_meta *meta_list = rqd.meta_list; + + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); + meta_list[i].lba = lba_list[paddr] = addr_empty; + } + + ret = pblk_submit_io_sync(pblk, &rqd); + if (ret) { + pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); + bio_put(bio); + goto free_ppa_list; + } + + atomic_dec(&pblk->inflight_io); + + if (rqd.error) { + pblk_log_write_err(pblk, &rqd); + ret = -EIO; + } + +free_ppa_list: + nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); + return ret; +} + +int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line, + void *emeta_buf) { struct nvm_tgt_dev *dev = pblk->dev; struct nvm_geo *geo = &dev->geo; @@ -635,24 +760,15 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, void *ppa_list, *meta_list; struct bio *bio; struct nvm_rq rqd; + u64 paddr = line->emeta_ssec; dma_addr_t dma_ppa_list, dma_meta_list; int min = pblk->min_write_pgs; int left_ppas = lm->emeta_sec[0]; - int id = line->id; + int line_id = line->id; int rq_ppas, rq_len; - int cmd_op, bio_op; int i, j; int ret; - if (dir == PBLK_WRITE) { - bio_op = REQ_OP_WRITE; - cmd_op = NVM_OP_PWRITE; - } else if (dir == PBLK_READ) { - bio_op = REQ_OP_READ; - cmd_op = NVM_OP_PREAD; - } else - return -EINVAL; - meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, &dma_meta_list); if (!meta_list) @@ -675,64 +791,43 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, } bio->bi_iter.bi_sector = 0; /* internal bio */ - bio_set_op_attrs(bio, bio_op, 0); + bio_set_op_attrs(bio, REQ_OP_READ, 0); rqd.bio = bio; rqd.meta_list = meta_list; rqd.ppa_list = ppa_list; rqd.dma_meta_list = dma_meta_list; rqd.dma_ppa_list = dma_ppa_list; - rqd.opcode = cmd_op; + rqd.opcode = NVM_OP_PREAD; rqd.nr_ppas = rq_ppas; - if (dir == PBLK_WRITE) { - struct pblk_sec_meta *meta_list = rqd.meta_list; + for (i = 0; i < rqd.nr_ppas; ) { + struct ppa_addr ppa = addr_to_gen_ppa(pblk, paddr, line_id); + int pos = pblk_ppa_to_pos(geo, ppa); - rqd.is_seq = 1; - for (i = 0; i < rqd.nr_ppas; ) { - spin_lock(&line->lock); - paddr = __pblk_alloc_page(pblk, line, min); - spin_unlock(&line->lock); - for (j = 0; j < min; j++, i++, paddr++) { - meta_list[i].lba = cpu_to_le64(ADDR_EMPTY); - rqd.ppa_list[i] = - addr_to_gen_ppa(pblk, paddr, id); - } - } - } else { - for (i = 0; i < rqd.nr_ppas; ) { - struct ppa_addr ppa = addr_to_gen_ppa(pblk, paddr, id); - int pos = pblk_ppa_to_pos(geo, ppa); + if (pblk_io_aligned(pblk, rq_ppas)) + rqd.is_seq = 1; - if (pblk_io_aligned(pblk, rq_ppas)) - rqd.is_seq = 1; - - while (test_bit(pos, line->blk_bitmap)) { - paddr += min; - if (pblk_boundary_paddr_checks(pblk, paddr)) { - pblk_err(pblk, "corrupt emeta line:%d\n", - line->id); - bio_put(bio); - ret = -EINTR; - goto free_rqd_dma; - } - - ppa = addr_to_gen_ppa(pblk, paddr, id); - pos = pblk_ppa_to_pos(geo, ppa); - } - - if (pblk_boundary_paddr_checks(pblk, paddr + min)) { - pblk_err(pblk, "corrupt emeta line:%d\n", - line->id); + while (test_bit(pos, line->blk_bitmap)) { + paddr += min; + if (pblk_boundary_paddr_checks(pblk, paddr)) { bio_put(bio); ret = -EINTR; goto free_rqd_dma; } - for (j = 0; j < min; j++, i++, paddr++) - rqd.ppa_list[i] = - addr_to_gen_ppa(pblk, paddr, line->id); + ppa = addr_to_gen_ppa(pblk, paddr, line_id); + pos = pblk_ppa_to_pos(geo, ppa); } + + if (pblk_boundary_paddr_checks(pblk, paddr + min)) { + bio_put(bio); + ret = -EINTR; + goto free_rqd_dma; + } + + for (j = 0; j < min; j++, i++, paddr++) + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line_id); } ret = pblk_submit_io_sync(pblk, &rqd); @@ -744,136 +839,19 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, atomic_dec(&pblk->inflight_io); - if (rqd.error) { - if (dir == PBLK_WRITE) - pblk_log_write_err(pblk, &rqd); - else - pblk_log_read_err(pblk, &rqd); - } + if (rqd.error) + pblk_log_read_err(pblk, &rqd); emeta_buf += rq_len; left_ppas -= rq_ppas; if (left_ppas) goto next_rq; + free_rqd_dma: nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); return ret; } -u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line) -{ - struct nvm_tgt_dev *dev = pblk->dev; - struct nvm_geo *geo = &dev->geo; - struct pblk_line_meta *lm = &pblk->lm; - int bit; - - /* This usually only happens on bad lines */ - bit = find_first_zero_bit(line->blk_bitmap, lm->blk_per_line); - if (bit >= lm->blk_per_line) - return -1; - - return bit * geo->ws_opt; -} - -static int pblk_line_submit_smeta_io(struct pblk *pblk, struct pblk_line *line, - u64 paddr, int dir) -{ - struct nvm_tgt_dev *dev = pblk->dev; - struct pblk_line_meta *lm = &pblk->lm; - struct bio *bio; - struct nvm_rq rqd; - __le64 *lba_list = NULL; - int i, ret; - int cmd_op, bio_op; - - if (dir == PBLK_WRITE) { - bio_op = REQ_OP_WRITE; - cmd_op = NVM_OP_PWRITE; - lba_list = emeta_to_lbas(pblk, line->emeta->buf); - } else if (dir == PBLK_READ_RECOV || dir == PBLK_READ) { - bio_op = REQ_OP_READ; - cmd_op = NVM_OP_PREAD; - } else - return -EINVAL; - - memset(&rqd, 0, sizeof(struct nvm_rq)); - - rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, - &rqd.dma_meta_list); - if (!rqd.meta_list) - return -ENOMEM; - - rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; - rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; - - bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); - if (IS_ERR(bio)) { - ret = PTR_ERR(bio); - goto free_ppa_list; - } - - bio->bi_iter.bi_sector = 0; /* internal bio */ - bio_set_op_attrs(bio, bio_op, 0); - - rqd.bio = bio; - rqd.opcode = cmd_op; - rqd.is_seq = 1; - rqd.nr_ppas = lm->smeta_sec; - - for (i = 0; i < lm->smeta_sec; i++, paddr++) { - struct pblk_sec_meta *meta_list = rqd.meta_list; - - rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); - - if (dir == PBLK_WRITE) { - __le64 addr_empty = cpu_to_le64(ADDR_EMPTY); - - meta_list[i].lba = lba_list[paddr] = addr_empty; - } - } - - /* - * This I/O is sent by the write thread when a line is replace. Since - * the write thread is the only one sending write and erase commands, - * there is no need to take the LUN semaphore. - */ - ret = pblk_submit_io_sync(pblk, &rqd); - if (ret) { - pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); - bio_put(bio); - goto free_ppa_list; - } - - atomic_dec(&pblk->inflight_io); - - if (rqd.error) { - if (dir == PBLK_WRITE) { - pblk_log_write_err(pblk, &rqd); - ret = 1; - } else if (dir == PBLK_READ) - pblk_log_read_err(pblk, &rqd); - } - -free_ppa_list: - nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); - - return ret; -} - -int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line) -{ - u64 bpaddr = pblk_line_smeta_start(pblk, line); - - return pblk_line_submit_smeta_io(pblk, line, bpaddr, PBLK_READ_RECOV); -} - -int pblk_line_read_emeta(struct pblk *pblk, struct pblk_line *line, - void *emeta_buf) -{ - return pblk_line_submit_emeta_io(pblk, line, emeta_buf, - line->emeta_ssec, PBLK_READ); -} - static void pblk_setup_e_rq(struct pblk *pblk, struct nvm_rq *rqd, struct ppa_addr ppa) { @@ -1102,7 +1080,7 @@ static int pblk_line_init_bb(struct pblk *pblk, struct pblk_line *line, line->smeta_ssec = off; line->cur_sec = off + lm->smeta_sec; - if (init && pblk_line_submit_smeta_io(pblk, line, off, PBLK_WRITE)) { + if (init && pblk_line_smeta_write(pblk, line, off)) { pblk_debug(pblk, "line smeta I/O failed. Retry\n"); return 0; } diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c index 157c2567c9e8..189d92b58447 100644 --- a/drivers/lightnvm/pblk-gc.c +++ b/drivers/lightnvm/pblk-gc.c @@ -144,7 +144,7 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk, if (!emeta_buf) return NULL; - ret = pblk_line_read_emeta(pblk, line, emeta_buf); + ret = pblk_line_emeta_read(pblk, line, emeta_buf); if (ret) { pblk_err(pblk, "line %d read emeta failed (%d)\n", line->id, ret); diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c index cf629ab016ba..7b27e958dc8e 100644 --- a/drivers/lightnvm/pblk-recovery.c +++ b/drivers/lightnvm/pblk-recovery.c @@ -835,7 +835,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk) continue; /* Lines that cannot be read are assumed as not written here */ - if (pblk_line_read_smeta(pblk, line)) + if (pblk_line_smeta_read(pblk, line)) continue; crc = pblk_calc_smeta_crc(pblk, smeta_buf); @@ -905,7 +905,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk) line->emeta = emeta; memset(line->emeta->buf, 0, lm->emeta_len[0]); - if (pblk_line_read_emeta(pblk, line, line->emeta->buf)) { + if (pblk_line_emeta_read(pblk, line, line->emeta->buf)) { pblk_recov_l2p_from_oob(pblk, line); goto next; } diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h index 48b3035df3c4..a71f9847957b 100644 --- a/drivers/lightnvm/pblk.h +++ b/drivers/lightnvm/pblk.h @@ -782,6 +782,7 @@ void pblk_log_write_err(struct pblk *pblk, struct nvm_rq *rqd); void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd); int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd); int pblk_submit_io_sync(struct pblk *pblk, struct nvm_rq *rqd); +int pblk_submit_io_sync_sem(struct pblk *pblk, struct nvm_rq *rqd); int pblk_submit_meta_io(struct pblk *pblk, struct pblk_line *meta_line); struct bio *pblk_bio_map_addr(struct pblk *pblk, void *data, unsigned int nr_secs, unsigned int len, @@ -806,8 +807,8 @@ void pblk_gen_run_ws(struct pblk *pblk, struct pblk_line *line, void *priv, void (*work)(struct work_struct *), gfp_t gfp_mask, struct workqueue_struct *wq); u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line); -int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line); -int pblk_line_read_emeta(struct pblk *pblk, struct pblk_line *line, +int pblk_line_smeta_read(struct pblk *pblk, struct pblk_line *line); +int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line, void *emeta_buf); int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr erase_ppa); void pblk_line_put(struct kref *ref); -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] lightnvm: pblk: refactor metadata paths 2018-08-17 10:19 ` [PATCH 1/2] lightnvm: pblk: refactor metadata paths Javier González @ 2018-08-21 11:53 ` Matias Bjørling 2018-08-21 11:56 ` Javier Gonzalez 0 siblings, 1 reply; 5+ messages in thread From: Matias Bjørling @ 2018-08-21 11:53 UTC (permalink / raw) To: javier; +Cc: axboe, linux-block, linux-kernel, javier On 08/17/2018 12:19 PM, Javier González wrote: > pblk maintains two different metadata paths for smeta and emeta, which > store metadata at the start of the line and at the end of the line, > respectively. Until now, these path has been common for writing and > retrieving metadata, however, as these paths diverge, the common code > becomes less clear and unnecessary complicated. > > In preparation for further changes to the metadata write path, this > patch separates the write and read paths for smeta and emeta and > removes the synchronous emeta path as it not used anymore (emeta is > scheduled asynchronously to prevent jittering due to internal I/Os). > > Signed-off-by: Javier González <javier@cnexlabs.com> > --- > drivers/lightnvm/pblk-core.c | 338 ++++++++++++++++++--------------------- > drivers/lightnvm/pblk-gc.c | 2 +- > drivers/lightnvm/pblk-recovery.c | 4 +- > drivers/lightnvm/pblk.h | 5 +- > 4 files changed, 164 insertions(+), 185 deletions(-) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index 72de7456845b..52306573cc0e 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -621,12 +621,137 @@ u64 pblk_lookup_page(struct pblk *pblk, struct pblk_line *line) > return paddr; > } > > -/* > - * Submit emeta to one LUN in the raid line at the time to avoid a deadlock when > - * taking the per LUN semaphore. > - */ > -static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, > - void *emeta_buf, u64 paddr, int dir) > +u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line) > +{ > + struct nvm_tgt_dev *dev = pblk->dev; > + struct nvm_geo *geo = &dev->geo; > + struct pblk_line_meta *lm = &pblk->lm; > + int bit; > + > + /* This usually only happens on bad lines */ > + bit = find_first_zero_bit(line->blk_bitmap, lm->blk_per_line); > + if (bit >= lm->blk_per_line) > + return -1; > + > + return bit * geo->ws_opt; > +} > + > +int pblk_line_smeta_read(struct pblk *pblk, struct pblk_line *line) > +{ > + struct nvm_tgt_dev *dev = pblk->dev; > + struct pblk_line_meta *lm = &pblk->lm; > + struct bio *bio; > + struct nvm_rq rqd; > + u64 paddr = pblk_line_smeta_start(pblk, line); > + int i, ret; > + > + memset(&rqd, 0, sizeof(struct nvm_rq)); > + > + rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, > + &rqd.dma_meta_list); > + if (!rqd.meta_list) > + return -ENOMEM; > + > + rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; > + rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; > + > + bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); > + if (IS_ERR(bio)) { > + ret = PTR_ERR(bio); > + goto free_ppa_list; > + } > + > + bio->bi_iter.bi_sector = 0; /* internal bio */ > + bio_set_op_attrs(bio, REQ_OP_READ, 0); > + > + rqd.bio = bio; > + rqd.opcode = NVM_OP_PREAD; > + rqd.nr_ppas = lm->smeta_sec; > + rqd.is_seq = 1; > + > + for (i = 0; i < lm->smeta_sec; i++, paddr++) > + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); > + > + ret = pblk_submit_io_sync(pblk, &rqd); > + if (ret) { > + pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); > + bio_put(bio); > + goto free_ppa_list; > + } > + > + atomic_dec(&pblk->inflight_io); > + > + if (rqd.error) > + pblk_log_read_err(pblk, &rqd); > + > +free_ppa_list: > + nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); > + return ret; > +} > + > +static int pblk_line_smeta_write(struct pblk *pblk, struct pblk_line *line, > + u64 paddr) > +{ > + struct nvm_tgt_dev *dev = pblk->dev; > + struct pblk_line_meta *lm = &pblk->lm; > + struct bio *bio; > + struct nvm_rq rqd; > + __le64 *lba_list = emeta_to_lbas(pblk, line->emeta->buf); > + __le64 addr_empty = cpu_to_le64(ADDR_EMPTY); > + int i, ret; > + > + memset(&rqd, 0, sizeof(struct nvm_rq)); > + > + rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, > + &rqd.dma_meta_list); > + if (!rqd.meta_list) > + return -ENOMEM; > + > + rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; > + rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; > + > + bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); > + if (IS_ERR(bio)) { > + ret = PTR_ERR(bio); > + goto free_ppa_list; > + } > + > + bio->bi_iter.bi_sector = 0; /* internal bio */ > + bio_set_op_attrs(bio, REQ_OP_WRITE, 0); > + > + rqd.bio = bio; > + rqd.opcode = NVM_OP_PWRITE; > + rqd.nr_ppas = lm->smeta_sec; > + rqd.is_seq = 1; > + > + for (i = 0; i < lm->smeta_sec; i++, paddr++) { > + struct pblk_sec_meta *meta_list = rqd.meta_list; > + > + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); > + meta_list[i].lba = lba_list[paddr] = addr_empty; > + } > + > + ret = pblk_submit_io_sync(pblk, &rqd); > + if (ret) { > + pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); > + bio_put(bio); > + goto free_ppa_list; > + } > + > + atomic_dec(&pblk->inflight_io); > + > + if (rqd.error) { > + pblk_log_write_err(pblk, &rqd); > + ret = -EIO; > + } > + > +free_ppa_list: > + nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); > + return ret; > +} > + > +int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line, > + void *emeta_buf) > { > struct nvm_tgt_dev *dev = pblk->dev; > struct nvm_geo *geo = &dev->geo; > @@ -635,24 +760,15 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, > void *ppa_list, *meta_list; > struct bio *bio; > struct nvm_rq rqd; > + u64 paddr = line->emeta_ssec; > dma_addr_t dma_ppa_list, dma_meta_list; > int min = pblk->min_write_pgs; > int left_ppas = lm->emeta_sec[0]; > - int id = line->id; > + int line_id = line->id; > int rq_ppas, rq_len; > - int cmd_op, bio_op; > int i, j; > int ret; > > - if (dir == PBLK_WRITE) { > - bio_op = REQ_OP_WRITE; > - cmd_op = NVM_OP_PWRITE; > - } else if (dir == PBLK_READ) { > - bio_op = REQ_OP_READ; > - cmd_op = NVM_OP_PREAD; > - } else > - return -EINVAL; > - > meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, > &dma_meta_list); > if (!meta_list) > @@ -675,64 +791,43 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, > } > > bio->bi_iter.bi_sector = 0; /* internal bio */ > - bio_set_op_attrs(bio, bio_op, 0); > + bio_set_op_attrs(bio, REQ_OP_READ, 0); > > rqd.bio = bio; > rqd.meta_list = meta_list; > rqd.ppa_list = ppa_list; > rqd.dma_meta_list = dma_meta_list; > rqd.dma_ppa_list = dma_ppa_list; > - rqd.opcode = cmd_op; > + rqd.opcode = NVM_OP_PREAD; > rqd.nr_ppas = rq_ppas; > > - if (dir == PBLK_WRITE) { > - struct pblk_sec_meta *meta_list = rqd.meta_list; > + for (i = 0; i < rqd.nr_ppas; ) { > + struct ppa_addr ppa = addr_to_gen_ppa(pblk, paddr, line_id); > + int pos = pblk_ppa_to_pos(geo, ppa); > > - rqd.is_seq = 1; > - for (i = 0; i < rqd.nr_ppas; ) { > - spin_lock(&line->lock); > - paddr = __pblk_alloc_page(pblk, line, min); > - spin_unlock(&line->lock); > - for (j = 0; j < min; j++, i++, paddr++) { > - meta_list[i].lba = cpu_to_le64(ADDR_EMPTY); > - rqd.ppa_list[i] = > - addr_to_gen_ppa(pblk, paddr, id); > - } > - } > - } else { > - for (i = 0; i < rqd.nr_ppas; ) { > - struct ppa_addr ppa = addr_to_gen_ppa(pblk, paddr, id); > - int pos = pblk_ppa_to_pos(geo, ppa); > + if (pblk_io_aligned(pblk, rq_ppas)) > + rqd.is_seq = 1; > > - if (pblk_io_aligned(pblk, rq_ppas)) > - rqd.is_seq = 1; > - > - while (test_bit(pos, line->blk_bitmap)) { > - paddr += min; > - if (pblk_boundary_paddr_checks(pblk, paddr)) { > - pblk_err(pblk, "corrupt emeta line:%d\n", > - line->id); > - bio_put(bio); > - ret = -EINTR; > - goto free_rqd_dma; > - } > - > - ppa = addr_to_gen_ppa(pblk, paddr, id); > - pos = pblk_ppa_to_pos(geo, ppa); > - } > - > - if (pblk_boundary_paddr_checks(pblk, paddr + min)) { > - pblk_err(pblk, "corrupt emeta line:%d\n", > - line->id); > + while (test_bit(pos, line->blk_bitmap)) { > + paddr += min; > + if (pblk_boundary_paddr_checks(pblk, paddr)) { > bio_put(bio); > ret = -EINTR; > goto free_rqd_dma; > } > > - for (j = 0; j < min; j++, i++, paddr++) > - rqd.ppa_list[i] = > - addr_to_gen_ppa(pblk, paddr, line->id); > + ppa = addr_to_gen_ppa(pblk, paddr, line_id); > + pos = pblk_ppa_to_pos(geo, ppa); > } > + > + if (pblk_boundary_paddr_checks(pblk, paddr + min)) { > + bio_put(bio); > + ret = -EINTR; > + goto free_rqd_dma; > + } > + > + for (j = 0; j < min; j++, i++, paddr++) > + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line_id); > } > > ret = pblk_submit_io_sync(pblk, &rqd); > @@ -744,136 +839,19 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, > > atomic_dec(&pblk->inflight_io); > > - if (rqd.error) { > - if (dir == PBLK_WRITE) > - pblk_log_write_err(pblk, &rqd); > - else > - pblk_log_read_err(pblk, &rqd); > - } > + if (rqd.error) > + pblk_log_read_err(pblk, &rqd); > > emeta_buf += rq_len; > left_ppas -= rq_ppas; > if (left_ppas) > goto next_rq; > + > free_rqd_dma: > nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); > return ret; > } > > -u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line) > -{ > - struct nvm_tgt_dev *dev = pblk->dev; > - struct nvm_geo *geo = &dev->geo; > - struct pblk_line_meta *lm = &pblk->lm; > - int bit; > - > - /* This usually only happens on bad lines */ > - bit = find_first_zero_bit(line->blk_bitmap, lm->blk_per_line); > - if (bit >= lm->blk_per_line) > - return -1; > - > - return bit * geo->ws_opt; > -} > - > -static int pblk_line_submit_smeta_io(struct pblk *pblk, struct pblk_line *line, > - u64 paddr, int dir) > -{ > - struct nvm_tgt_dev *dev = pblk->dev; > - struct pblk_line_meta *lm = &pblk->lm; > - struct bio *bio; > - struct nvm_rq rqd; > - __le64 *lba_list = NULL; > - int i, ret; > - int cmd_op, bio_op; > - > - if (dir == PBLK_WRITE) { > - bio_op = REQ_OP_WRITE; > - cmd_op = NVM_OP_PWRITE; > - lba_list = emeta_to_lbas(pblk, line->emeta->buf); > - } else if (dir == PBLK_READ_RECOV || dir == PBLK_READ) { > - bio_op = REQ_OP_READ; > - cmd_op = NVM_OP_PREAD; > - } else > - return -EINVAL; > - > - memset(&rqd, 0, sizeof(struct nvm_rq)); > - > - rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, > - &rqd.dma_meta_list); > - if (!rqd.meta_list) > - return -ENOMEM; > - > - rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; > - rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; > - > - bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); > - if (IS_ERR(bio)) { > - ret = PTR_ERR(bio); > - goto free_ppa_list; > - } > - > - bio->bi_iter.bi_sector = 0; /* internal bio */ > - bio_set_op_attrs(bio, bio_op, 0); > - > - rqd.bio = bio; > - rqd.opcode = cmd_op; > - rqd.is_seq = 1; > - rqd.nr_ppas = lm->smeta_sec; > - > - for (i = 0; i < lm->smeta_sec; i++, paddr++) { > - struct pblk_sec_meta *meta_list = rqd.meta_list; > - > - rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); > - > - if (dir == PBLK_WRITE) { > - __le64 addr_empty = cpu_to_le64(ADDR_EMPTY); > - > - meta_list[i].lba = lba_list[paddr] = addr_empty; > - } > - } > - > - /* > - * This I/O is sent by the write thread when a line is replace. Since > - * the write thread is the only one sending write and erase commands, > - * there is no need to take the LUN semaphore. > - */ > - ret = pblk_submit_io_sync(pblk, &rqd); > - if (ret) { > - pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); > - bio_put(bio); > - goto free_ppa_list; > - } > - > - atomic_dec(&pblk->inflight_io); > - > - if (rqd.error) { > - if (dir == PBLK_WRITE) { > - pblk_log_write_err(pblk, &rqd); > - ret = 1; > - } else if (dir == PBLK_READ) > - pblk_log_read_err(pblk, &rqd); > - } > - > -free_ppa_list: > - nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); > - > - return ret; > -} > - > -int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line) > -{ > - u64 bpaddr = pblk_line_smeta_start(pblk, line); > - > - return pblk_line_submit_smeta_io(pblk, line, bpaddr, PBLK_READ_RECOV); > -} > - > -int pblk_line_read_emeta(struct pblk *pblk, struct pblk_line *line, > - void *emeta_buf) > -{ > - return pblk_line_submit_emeta_io(pblk, line, emeta_buf, > - line->emeta_ssec, PBLK_READ); > -} > - > static void pblk_setup_e_rq(struct pblk *pblk, struct nvm_rq *rqd, > struct ppa_addr ppa) > { > @@ -1102,7 +1080,7 @@ static int pblk_line_init_bb(struct pblk *pblk, struct pblk_line *line, > line->smeta_ssec = off; > line->cur_sec = off + lm->smeta_sec; > > - if (init && pblk_line_submit_smeta_io(pblk, line, off, PBLK_WRITE)) { > + if (init && pblk_line_smeta_write(pblk, line, off)) { > pblk_debug(pblk, "line smeta I/O failed. Retry\n"); > return 0; > } > diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c > index 157c2567c9e8..189d92b58447 100644 > --- a/drivers/lightnvm/pblk-gc.c > +++ b/drivers/lightnvm/pblk-gc.c > @@ -144,7 +144,7 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk, > if (!emeta_buf) > return NULL; > > - ret = pblk_line_read_emeta(pblk, line, emeta_buf); > + ret = pblk_line_emeta_read(pblk, line, emeta_buf); > if (ret) { > pblk_err(pblk, "line %d read emeta failed (%d)\n", > line->id, ret); > diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c > index cf629ab016ba..7b27e958dc8e 100644 > --- a/drivers/lightnvm/pblk-recovery.c > +++ b/drivers/lightnvm/pblk-recovery.c > @@ -835,7 +835,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk) > continue; > > /* Lines that cannot be read are assumed as not written here */ > - if (pblk_line_read_smeta(pblk, line)) > + if (pblk_line_smeta_read(pblk, line)) > continue; > > crc = pblk_calc_smeta_crc(pblk, smeta_buf); > @@ -905,7 +905,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk) > line->emeta = emeta; > memset(line->emeta->buf, 0, lm->emeta_len[0]); > > - if (pblk_line_read_emeta(pblk, line, line->emeta->buf)) { > + if (pblk_line_emeta_read(pblk, line, line->emeta->buf)) { > pblk_recov_l2p_from_oob(pblk, line); > goto next; > } > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h > index 48b3035df3c4..a71f9847957b 100644 > --- a/drivers/lightnvm/pblk.h > +++ b/drivers/lightnvm/pblk.h > @@ -782,6 +782,7 @@ void pblk_log_write_err(struct pblk *pblk, struct nvm_rq *rqd); > void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd); > int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd); > int pblk_submit_io_sync(struct pblk *pblk, struct nvm_rq *rqd); > +int pblk_submit_io_sync_sem(struct pblk *pblk, struct nvm_rq *rqd); I'll remove this from the patch and add to the next. > int pblk_submit_meta_io(struct pblk *pblk, struct pblk_line *meta_line); > struct bio *pblk_bio_map_addr(struct pblk *pblk, void *data, > unsigned int nr_secs, unsigned int len, > @@ -806,8 +807,8 @@ void pblk_gen_run_ws(struct pblk *pblk, struct pblk_line *line, void *priv, > void (*work)(struct work_struct *), gfp_t gfp_mask, > struct workqueue_struct *wq); > u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line); > -int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line); > -int pblk_line_read_emeta(struct pblk *pblk, struct pblk_line *line, > +int pblk_line_smeta_read(struct pblk *pblk, struct pblk_line *line); > +int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line, > void *emeta_buf); > int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr erase_ppa); > void pblk_line_put(struct kref *ref); > Thanks. Applied for 4.20. I think the split could be optimized quite a bit with respect to complexity. Now a lot of very similar code is spread across two functions. I may look into optimizing the request alloc path, such that it not open coded in most places. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] lightnvm: pblk: refactor metadata paths 2018-08-21 11:53 ` Matias Bjørling @ 2018-08-21 11:56 ` Javier Gonzalez 0 siblings, 0 replies; 5+ messages in thread From: Javier Gonzalez @ 2018-08-21 11:56 UTC (permalink / raw) To: Matias Bjørling; +Cc: javier, axboe, linux-block, linux-kernel > On 21 Aug 2018, at 13.54, Matias Bjørling <mb@lightnvm.io> wrote: > >> On 08/17/2018 12:19 PM, Javier González wrote: >> pblk maintains two different metadata paths for smeta and emeta, which >> store metadata at the start of the line and at the end of the line, >> respectively. Until now, these path has been common for writing and >> retrieving metadata, however, as these paths diverge, the common code >> becomes less clear and unnecessary complicated. >> In preparation for further changes to the metadata write path, this >> patch separates the write and read paths for smeta and emeta and >> removes the synchronous emeta path as it not used anymore (emeta is >> scheduled asynchronously to prevent jittering due to internal I/Os). >> Signed-off-by: Javier González <javier@cnexlabs.com> >> --- >> drivers/lightnvm/pblk-core.c | 338 ++++++++++++++++++--------------------- >> drivers/lightnvm/pblk-gc.c | 2 +- >> drivers/lightnvm/pblk-recovery.c | 4 +- >> drivers/lightnvm/pblk.h | 5 +- >> 4 files changed, 164 insertions(+), 185 deletions(-) >> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c >> index 72de7456845b..52306573cc0e 100644 >> --- a/drivers/lightnvm/pblk-core.c >> +++ b/drivers/lightnvm/pblk-core.c >> @@ -621,12 +621,137 @@ u64 pblk_lookup_page(struct pblk *pblk, struct pblk_line *line) >> return paddr; >> } >> -/* >> - * Submit emeta to one LUN in the raid line at the time to avoid a deadlock when >> - * taking the per LUN semaphore. >> - */ >> -static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, >> - void *emeta_buf, u64 paddr, int dir) >> +u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line) >> +{ >> + struct nvm_tgt_dev *dev = pblk->dev; >> + struct nvm_geo *geo = &dev->geo; >> + struct pblk_line_meta *lm = &pblk->lm; >> + int bit; >> + >> + /* This usually only happens on bad lines */ >> + bit = find_first_zero_bit(line->blk_bitmap, lm->blk_per_line); >> + if (bit >= lm->blk_per_line) >> + return -1; >> + >> + return bit * geo->ws_opt; >> +} >> + >> +int pblk_line_smeta_read(struct pblk *pblk, struct pblk_line *line) >> +{ >> + struct nvm_tgt_dev *dev = pblk->dev; >> + struct pblk_line_meta *lm = &pblk->lm; >> + struct bio *bio; >> + struct nvm_rq rqd; >> + u64 paddr = pblk_line_smeta_start(pblk, line); >> + int i, ret; >> + >> + memset(&rqd, 0, sizeof(struct nvm_rq)); >> + >> + rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, >> + &rqd.dma_meta_list); >> + if (!rqd.meta_list) >> + return -ENOMEM; >> + >> + rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; >> + rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; >> + >> + bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); >> + if (IS_ERR(bio)) { >> + ret = PTR_ERR(bio); >> + goto free_ppa_list; >> + } >> + >> + bio->bi_iter.bi_sector = 0; /* internal bio */ >> + bio_set_op_attrs(bio, REQ_OP_READ, 0); >> + >> + rqd.bio = bio; >> + rqd.opcode = NVM_OP_PREAD; >> + rqd.nr_ppas = lm->smeta_sec; >> + rqd.is_seq = 1; >> + >> + for (i = 0; i < lm->smeta_sec; i++, paddr++) >> + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); >> + >> + ret = pblk_submit_io_sync(pblk, &rqd); >> + if (ret) { >> + pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); >> + bio_put(bio); >> + goto free_ppa_list; >> + } >> + >> + atomic_dec(&pblk->inflight_io); >> + >> + if (rqd.error) >> + pblk_log_read_err(pblk, &rqd); >> + >> +free_ppa_list: >> + nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); >> + return ret; >> +} >> + >> +static int pblk_line_smeta_write(struct pblk *pblk, struct pblk_line *line, >> + u64 paddr) >> +{ >> + struct nvm_tgt_dev *dev = pblk->dev; >> + struct pblk_line_meta *lm = &pblk->lm; >> + struct bio *bio; >> + struct nvm_rq rqd; >> + __le64 *lba_list = emeta_to_lbas(pblk, line->emeta->buf); >> + __le64 addr_empty = cpu_to_le64(ADDR_EMPTY); >> + int i, ret; >> + >> + memset(&rqd, 0, sizeof(struct nvm_rq)); >> + >> + rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, >> + &rqd.dma_meta_list); >> + if (!rqd.meta_list) >> + return -ENOMEM; >> + >> + rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; >> + rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; >> + >> + bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); >> + if (IS_ERR(bio)) { >> + ret = PTR_ERR(bio); >> + goto free_ppa_list; >> + } >> + >> + bio->bi_iter.bi_sector = 0; /* internal bio */ >> + bio_set_op_attrs(bio, REQ_OP_WRITE, 0); >> + >> + rqd.bio = bio; >> + rqd.opcode = NVM_OP_PWRITE; >> + rqd.nr_ppas = lm->smeta_sec; >> + rqd.is_seq = 1; >> + >> + for (i = 0; i < lm->smeta_sec; i++, paddr++) { >> + struct pblk_sec_meta *meta_list = rqd.meta_list; >> + >> + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); >> + meta_list[i].lba = lba_list[paddr] = addr_empty; >> + } >> + >> + ret = pblk_submit_io_sync(pblk, &rqd); >> + if (ret) { >> + pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); >> + bio_put(bio); >> + goto free_ppa_list; >> + } >> + >> + atomic_dec(&pblk->inflight_io); >> + >> + if (rqd.error) { >> + pblk_log_write_err(pblk, &rqd); >> + ret = -EIO; >> + } >> + >> +free_ppa_list: >> + nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); >> + return ret; >> +} >> + >> +int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line, >> + void *emeta_buf) >> { >> struct nvm_tgt_dev *dev = pblk->dev; >> struct nvm_geo *geo = &dev->geo; >> @@ -635,24 +760,15 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, >> void *ppa_list, *meta_list; >> struct bio *bio; >> struct nvm_rq rqd; >> + u64 paddr = line->emeta_ssec; >> dma_addr_t dma_ppa_list, dma_meta_list; >> int min = pblk->min_write_pgs; >> int left_ppas = lm->emeta_sec[0]; >> - int id = line->id; >> + int line_id = line->id; >> int rq_ppas, rq_len; >> - int cmd_op, bio_op; >> int i, j; >> int ret; >> - if (dir == PBLK_WRITE) { >> - bio_op = REQ_OP_WRITE; >> - cmd_op = NVM_OP_PWRITE; >> - } else if (dir == PBLK_READ) { >> - bio_op = REQ_OP_READ; >> - cmd_op = NVM_OP_PREAD; >> - } else >> - return -EINVAL; >> - >> meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, >> &dma_meta_list); >> if (!meta_list) >> @@ -675,64 +791,43 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, >> } >> bio->bi_iter.bi_sector = 0; /* internal bio */ >> - bio_set_op_attrs(bio, bio_op, 0); >> + bio_set_op_attrs(bio, REQ_OP_READ, 0); >> rqd.bio = bio; >> rqd.meta_list = meta_list; >> rqd.ppa_list = ppa_list; >> rqd.dma_meta_list = dma_meta_list; >> rqd.dma_ppa_list = dma_ppa_list; >> - rqd.opcode = cmd_op; >> + rqd.opcode = NVM_OP_PREAD; >> rqd.nr_ppas = rq_ppas; >> - if (dir == PBLK_WRITE) { >> - struct pblk_sec_meta *meta_list = rqd.meta_list; >> + for (i = 0; i < rqd.nr_ppas; ) { >> + struct ppa_addr ppa = addr_to_gen_ppa(pblk, paddr, line_id); >> + int pos = pblk_ppa_to_pos(geo, ppa); >> - rqd.is_seq = 1; >> - for (i = 0; i < rqd.nr_ppas; ) { >> - spin_lock(&line->lock); >> - paddr = __pblk_alloc_page(pblk, line, min); >> - spin_unlock(&line->lock); >> - for (j = 0; j < min; j++, i++, paddr++) { >> - meta_list[i].lba = cpu_to_le64(ADDR_EMPTY); >> - rqd.ppa_list[i] = >> - addr_to_gen_ppa(pblk, paddr, id); >> - } >> - } >> - } else { >> - for (i = 0; i < rqd.nr_ppas; ) { >> - struct ppa_addr ppa = addr_to_gen_ppa(pblk, paddr, id); >> - int pos = pblk_ppa_to_pos(geo, ppa); >> + if (pblk_io_aligned(pblk, rq_ppas)) >> + rqd.is_seq = 1; >> - if (pblk_io_aligned(pblk, rq_ppas)) >> - rqd.is_seq = 1; >> - >> - while (test_bit(pos, line->blk_bitmap)) { >> - paddr += min; >> - if (pblk_boundary_paddr_checks(pblk, paddr)) { >> - pblk_err(pblk, "corrupt emeta line:%d\n", >> - line->id); >> - bio_put(bio); >> - ret = -EINTR; >> - goto free_rqd_dma; >> - } >> - >> - ppa = addr_to_gen_ppa(pblk, paddr, id); >> - pos = pblk_ppa_to_pos(geo, ppa); >> - } >> - >> - if (pblk_boundary_paddr_checks(pblk, paddr + min)) { >> - pblk_err(pblk, "corrupt emeta line:%d\n", >> - line->id); >> + while (test_bit(pos, line->blk_bitmap)) { >> + paddr += min; >> + if (pblk_boundary_paddr_checks(pblk, paddr)) { >> bio_put(bio); >> ret = -EINTR; >> goto free_rqd_dma; >> } >> - for (j = 0; j < min; j++, i++, paddr++) >> - rqd.ppa_list[i] = >> - addr_to_gen_ppa(pblk, paddr, line->id); >> + ppa = addr_to_gen_ppa(pblk, paddr, line_id); >> + pos = pblk_ppa_to_pos(geo, ppa); >> } >> + >> + if (pblk_boundary_paddr_checks(pblk, paddr + min)) { >> + bio_put(bio); >> + ret = -EINTR; >> + goto free_rqd_dma; >> + } >> + >> + for (j = 0; j < min; j++, i++, paddr++) >> + rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line_id); >> } >> ret = pblk_submit_io_sync(pblk, &rqd); >> @@ -744,136 +839,19 @@ static int pblk_line_submit_emeta_io(struct pblk *pblk, struct pblk_line *line, >> atomic_dec(&pblk->inflight_io); >> - if (rqd.error) { >> - if (dir == PBLK_WRITE) >> - pblk_log_write_err(pblk, &rqd); >> - else >> - pblk_log_read_err(pblk, &rqd); >> - } >> + if (rqd.error) >> + pblk_log_read_err(pblk, &rqd); >> emeta_buf += rq_len; >> left_ppas -= rq_ppas; >> if (left_ppas) >> goto next_rq; >> + >> free_rqd_dma: >> nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); >> return ret; >> } >> -u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line) >> -{ >> - struct nvm_tgt_dev *dev = pblk->dev; >> - struct nvm_geo *geo = &dev->geo; >> - struct pblk_line_meta *lm = &pblk->lm; >> - int bit; >> - >> - /* This usually only happens on bad lines */ >> - bit = find_first_zero_bit(line->blk_bitmap, lm->blk_per_line); >> - if (bit >= lm->blk_per_line) >> - return -1; >> - >> - return bit * geo->ws_opt; >> -} >> - >> -static int pblk_line_submit_smeta_io(struct pblk *pblk, struct pblk_line *line, >> - u64 paddr, int dir) >> -{ >> - struct nvm_tgt_dev *dev = pblk->dev; >> - struct pblk_line_meta *lm = &pblk->lm; >> - struct bio *bio; >> - struct nvm_rq rqd; >> - __le64 *lba_list = NULL; >> - int i, ret; >> - int cmd_op, bio_op; >> - >> - if (dir == PBLK_WRITE) { >> - bio_op = REQ_OP_WRITE; >> - cmd_op = NVM_OP_PWRITE; >> - lba_list = emeta_to_lbas(pblk, line->emeta->buf); >> - } else if (dir == PBLK_READ_RECOV || dir == PBLK_READ) { >> - bio_op = REQ_OP_READ; >> - cmd_op = NVM_OP_PREAD; >> - } else >> - return -EINVAL; >> - >> - memset(&rqd, 0, sizeof(struct nvm_rq)); >> - >> - rqd.meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, >> - &rqd.dma_meta_list); >> - if (!rqd.meta_list) >> - return -ENOMEM; >> - >> - rqd.ppa_list = rqd.meta_list + pblk_dma_meta_size; >> - rqd.dma_ppa_list = rqd.dma_meta_list + pblk_dma_meta_size; >> - >> - bio = bio_map_kern(dev->q, line->smeta, lm->smeta_len, GFP_KERNEL); >> - if (IS_ERR(bio)) { >> - ret = PTR_ERR(bio); >> - goto free_ppa_list; >> - } >> - >> - bio->bi_iter.bi_sector = 0; /* internal bio */ >> - bio_set_op_attrs(bio, bio_op, 0); >> - >> - rqd.bio = bio; >> - rqd.opcode = cmd_op; >> - rqd.is_seq = 1; >> - rqd.nr_ppas = lm->smeta_sec; >> - >> - for (i = 0; i < lm->smeta_sec; i++, paddr++) { >> - struct pblk_sec_meta *meta_list = rqd.meta_list; >> - >> - rqd.ppa_list[i] = addr_to_gen_ppa(pblk, paddr, line->id); >> - >> - if (dir == PBLK_WRITE) { >> - __le64 addr_empty = cpu_to_le64(ADDR_EMPTY); >> - >> - meta_list[i].lba = lba_list[paddr] = addr_empty; >> - } >> - } >> - >> - /* >> - * This I/O is sent by the write thread when a line is replace. Since >> - * the write thread is the only one sending write and erase commands, >> - * there is no need to take the LUN semaphore. >> - */ >> - ret = pblk_submit_io_sync(pblk, &rqd); >> - if (ret) { >> - pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); >> - bio_put(bio); >> - goto free_ppa_list; >> - } >> - >> - atomic_dec(&pblk->inflight_io); >> - >> - if (rqd.error) { >> - if (dir == PBLK_WRITE) { >> - pblk_log_write_err(pblk, &rqd); >> - ret = 1; >> - } else if (dir == PBLK_READ) >> - pblk_log_read_err(pblk, &rqd); >> - } >> - >> -free_ppa_list: >> - nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list); >> - >> - return ret; >> -} >> - >> -int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line) >> -{ >> - u64 bpaddr = pblk_line_smeta_start(pblk, line); >> - >> - return pblk_line_submit_smeta_io(pblk, line, bpaddr, PBLK_READ_RECOV); >> -} >> - >> -int pblk_line_read_emeta(struct pblk *pblk, struct pblk_line *line, >> - void *emeta_buf) >> -{ >> - return pblk_line_submit_emeta_io(pblk, line, emeta_buf, >> - line->emeta_ssec, PBLK_READ); >> -} >> - >> static void pblk_setup_e_rq(struct pblk *pblk, struct nvm_rq *rqd, >> struct ppa_addr ppa) >> { >> @@ -1102,7 +1080,7 @@ static int pblk_line_init_bb(struct pblk *pblk, struct pblk_line *line, >> line->smeta_ssec = off; >> line->cur_sec = off + lm->smeta_sec; >> - if (init && pblk_line_submit_smeta_io(pblk, line, off, PBLK_WRITE)) { >> + if (init && pblk_line_smeta_write(pblk, line, off)) { >> pblk_debug(pblk, "line smeta I/O failed. Retry\n"); >> return 0; >> } >> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c >> index 157c2567c9e8..189d92b58447 100644 >> --- a/drivers/lightnvm/pblk-gc.c >> +++ b/drivers/lightnvm/pblk-gc.c >> @@ -144,7 +144,7 @@ static __le64 *get_lba_list_from_emeta(struct pblk *pblk, >> if (!emeta_buf) >> return NULL; >> - ret = pblk_line_read_emeta(pblk, line, emeta_buf); >> + ret = pblk_line_emeta_read(pblk, line, emeta_buf); >> if (ret) { >> pblk_err(pblk, "line %d read emeta failed (%d)\n", >> line->id, ret); >> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c >> index cf629ab016ba..7b27e958dc8e 100644 >> --- a/drivers/lightnvm/pblk-recovery.c >> +++ b/drivers/lightnvm/pblk-recovery.c >> @@ -835,7 +835,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk) >> continue; >> /* Lines that cannot be read are assumed as not written here */ >> - if (pblk_line_read_smeta(pblk, line)) >> + if (pblk_line_smeta_read(pblk, line)) >> continue; >> crc = pblk_calc_smeta_crc(pblk, smeta_buf); >> @@ -905,7 +905,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk) >> line->emeta = emeta; >> memset(line->emeta->buf, 0, lm->emeta_len[0]); >> - if (pblk_line_read_emeta(pblk, line, line->emeta->buf)) { >> + if (pblk_line_emeta_read(pblk, line, line->emeta->buf)) { >> pblk_recov_l2p_from_oob(pblk, line); >> goto next; >> } >> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h >> index 48b3035df3c4..a71f9847957b 100644 >> --- a/drivers/lightnvm/pblk.h >> +++ b/drivers/lightnvm/pblk.h >> @@ -782,6 +782,7 @@ void pblk_log_write_err(struct pblk *pblk, struct nvm_rq *rqd); >> void pblk_log_read_err(struct pblk *pblk, struct nvm_rq *rqd); >> int pblk_submit_io(struct pblk *pblk, struct nvm_rq *rqd); >> int pblk_submit_io_sync(struct pblk *pblk, struct nvm_rq *rqd); >> +int pblk_submit_io_sync_sem(struct pblk *pblk, struct nvm_rq *rqd); > > I'll remove this from the patch and add to the next. > >> int pblk_submit_meta_io(struct pblk *pblk, struct pblk_line *meta_line); >> struct bio *pblk_bio_map_addr(struct pblk *pblk, void *data, >> unsigned int nr_secs, unsigned int len, >> @@ -806,8 +807,8 @@ void pblk_gen_run_ws(struct pblk *pblk, struct pblk_line *line, void *priv, >> void (*work)(struct work_struct *), gfp_t gfp_mask, >> struct workqueue_struct *wq); >> u64 pblk_line_smeta_start(struct pblk *pblk, struct pblk_line *line); >> -int pblk_line_read_smeta(struct pblk *pblk, struct pblk_line *line); >> -int pblk_line_read_emeta(struct pblk *pblk, struct pblk_line *line, >> +int pblk_line_smeta_read(struct pblk *pblk, struct pblk_line *line); >> +int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line, >> void *emeta_buf); >> int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr erase_ppa); >> void pblk_line_put(struct kref *ref); > > Thanks. Applied for 4.20. I think the split could be optimized quite a bit with respect to complexity. Now a lot of very similar code is spread across two functions. I may look into optimizing the request alloc path, such that it not open coded in most places. Let me take another round at it. I’ll submit a V2 later this week. Javier. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] lightnvm: pblk: take write semaphore on metadata 2018-08-17 10:19 [V3 PATCH 0/2] lightnvm: pblk: take write semaphore on metadata Javier González 2018-08-17 10:19 ` [PATCH 1/2] lightnvm: pblk: refactor metadata paths Javier González @ 2018-08-17 10:19 ` Javier González 1 sibling, 0 replies; 5+ messages in thread From: Javier González @ 2018-08-17 10:19 UTC (permalink / raw) To: mb; +Cc: axboe, linux-block, linux-kernel, Javier González pblk guarantees write ordering at a chunk level through a per open chunk semaphore. At this point, since we only have an open I/O stream for both user and GC data, the semaphore is per parallel unit. For the metadata I/O that is synchronous, the semaphore is not needed as ordering is guaranteed. However, if the metadata scheme changes or multiple streams are open, this guarantee might not be preserved. This patch makes sure that all writes go through the semaphore, even for synchronous I/O. This is consistent with pblk's write I/O model. It also simplifies maintenance since changes in the metadata scheme could cause ordering issues. Signed-off-by: Javier González <javier@cnexlabs.com> --- drivers/lightnvm/pblk-core.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c index 52306573cc0e..eac5bd551860 100644 --- a/drivers/lightnvm/pblk-core.c +++ b/drivers/lightnvm/pblk-core.c @@ -493,6 +493,20 @@ int pblk_submit_io_sync(struct pblk *pblk, struct nvm_rq *rqd) return nvm_submit_io_sync(dev, rqd); } +int pblk_submit_io_sync_sem(struct pblk *pblk, struct nvm_rq *rqd) +{ + struct ppa_addr *ppa_list; + int ret; + + ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr; + + pblk_down_page(pblk, ppa_list, rqd->nr_ppas); + ret = pblk_submit_io_sync(pblk, rqd); + pblk_up_page(pblk, ppa_list, rqd->nr_ppas); + + return ret; +} + static void pblk_bio_map_addr_endio(struct bio *bio) { bio_put(bio); @@ -731,7 +745,7 @@ static int pblk_line_smeta_write(struct pblk *pblk, struct pblk_line *line, meta_list[i].lba = lba_list[paddr] = addr_empty; } - ret = pblk_submit_io_sync(pblk, &rqd); + ret = pblk_submit_io_sync_sem(pblk, &rqd); if (ret) { pblk_err(pblk, "smeta I/O submission failed: %d\n", ret); bio_put(bio); -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-21 11:56 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-17 10:19 [V3 PATCH 0/2] lightnvm: pblk: take write semaphore on metadata Javier González 2018-08-17 10:19 ` [PATCH 1/2] lightnvm: pblk: refactor metadata paths Javier González 2018-08-21 11:53 ` Matias Bjørling 2018-08-21 11:56 ` Javier Gonzalez 2018-08-17 10:19 ` [PATCH 2/2] lightnvm: pblk: take write semaphore on metadata Javier González
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).