On 08/29/2018 03:41 PM, Javier Gonzalez wrote:
On 29 Aug 2018, at 15.36, Matias Bjørling <mb@xxxxxxxxxxx> wrote:
On 08/29/2018 03:18 PM, Javier Gonzalez wrote:
On 29 Aug 2018, at 15.00, Matias Bjørling <mb@xxxxxxxxxxx> wrote:
On 08/29/2018 10:56 AM, Javier González wrote:
dma allocations for ppa_list and meta_list in rqd are replicated in
several places across the pblk codebase. Make helpers to encapsulate
creation and deletion to simplify the code.
Signed-off-by: Javier González <javier@xxxxxxxxxxxx>
---
drivers/lightnvm/pblk-core.c | 69 +++++++++++++++++++++++++---------------
drivers/lightnvm/pblk-read.c | 35 ++++++++++----------
drivers/lightnvm/pblk-recovery.c | 29 ++++++-----------
drivers/lightnvm/pblk-write.c | 15 ++-------
drivers/lightnvm/pblk.h | 3 ++
5 files changed, 74 insertions(+), 77 deletions(-)
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 09160ec02c5f..767178185f19 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -237,6 +237,34 @@ static void pblk_invalidate_range(struct pblk *pblk, sector_t slba,
spin_unlock(&pblk->trans_lock);
}
+int pblk_setup_rqd(struct pblk *pblk, struct nvm_rq *rqd, gfp_t mem_flags,
+ bool is_vector)
The mem_flags argument can be removed. It is GFP_KERNEL from all the
places it is called.
Thought it was better to have the flexibility in a helper function, but
we can always add it later on if needed...
is_vector, will be better to do nr_ppas (or similar name). Then
pblk_submit_read/pblk_submit_read_gc are a bit cleaner.
We can do that too, yes.
+{
+ struct nvm_tgt_dev *dev = pblk->dev;
+
+ rqd->meta_list = nvm_dev_dma_alloc(dev->parent, mem_flags,
+ &rqd->dma_meta_list);
+ if (!rqd->meta_list)
+ return -ENOMEM;
+
+ if (!is_vector)
+ return 0;
+
+ rqd->ppa_list = rqd->meta_list + pblk_dma_meta_size;
+ rqd->dma_ppa_list = rqd->dma_meta_list + pblk_dma_meta_size;
Wrt to is_vector, does it matter if we just set ppa_list and
dma_ppa_list? If we have them, we use them, else leave them alone?
If we only have 1 address then ppa_addr is set and the ppa_list attempt
to free in the completion path interpreting ppa_addr as the dma address.
So I don't think so - unless I'm missing something?
In that case, we could drop is_vector/nr_ppas all together? That would be nice.
The problem is that the metadata region still needs to be used, even if
the ppa_list is not set. Thing that the oob area can be larger than
64bits, so we cannot do the dma address is the actual value trick.
So if encapsulating, we need to know if we need to allocate the ppa_list
or not.
Does it make sense?
Cool. We'll just leave it as is.
