diff mbox series

lightnvm: pblk: refactor put line fn on read completion

Message ID 20180820114320.3764-1-mb@lightnvm.io (mailing list archive)
State New, archived
Headers show
Series lightnvm: pblk: refactor put line fn on read completion | expand

Commit Message

Matias Bjorling Aug. 20, 2018, 11:43 a.m. UTC
The read completion path uses the put_line variable to decide whether
the reference on a line should be released. The function name used for
that is pblk_read_put_rqd_kref, which could lead one to believe that it
is the rqd that is releasing the reference, while it is the line
reference that is put.

Rename and also split the function in two to account for either rqd or
single ppa callers.

Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-read.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Comments

Javier Gonzalez Aug. 20, 2018, 11:49 a.m. UTC | #1
> On 20 Aug 2018, at 13.43, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> The read completion path uses the put_line variable to decide whether
> the reference on a line should be released. The function name used for
> that is pblk_read_put_rqd_kref, which could lead one to believe that it
> is the rqd that is releasing the reference, while it is the line
> reference that is put.
> 
> Rename and also split the function in two to account for either rqd or
> single ppa callers.
> 
> Signed-off-by: Matias Bjørling <mb@lightnvm.io>
> ---
> drivers/lightnvm/pblk-read.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
> index cd2f61eed6a0..16858eaf694a 100644
> --- a/drivers/lightnvm/pblk-read.c
> +++ b/drivers/lightnvm/pblk-read.c
> @@ -165,20 +165,23 @@ static void pblk_read_check_rand(struct pblk *pblk, struct nvm_rq *rqd,
> 	WARN_ONCE(j != rqd->nr_ppas, "pblk: corrupted random request\n");
> }
> 
> -static void pblk_read_put_rqd_kref(struct pblk *pblk, struct nvm_rq *rqd)
> +static void __pblk_read_put_line(struct pblk *pblk, struct ppa_addr ppa)
> {
> -	struct ppa_addr *ppa_list;
> -	int i;
> -
> -	ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
> -
> -	for (i = 0; i < rqd->nr_ppas; i++) {
> -		struct ppa_addr ppa = ppa_list[i];
> 		struct pblk_line *line;
> 
> 		line = &pblk->lines[pblk_ppa_to_line(ppa)];
> 		kref_put(&line->ref, pblk_line_put_wq);
> -	}
> +}
> +
> +static void pblk_read_put_line(struct pblk *pblk, struct nvm_rq *rqd)
> +{
> +	struct ppa_addr *ppa_list;
> +	int i;
> +
> +	ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
> +
> +	for (i = 0; i < rqd->nr_ppas; i++)
> +		__pblk_read_put_line(pblk, ppa_list[i]);
> }
> 
> static void pblk_end_user_read(struct bio *bio)
> @@ -208,7 +211,7 @@ static void __pblk_end_io_read(struct pblk *pblk, struct nvm_rq *rqd,
> 		bio_put(int_bio);
> 
> 	if (put_line)
> -		pblk_read_put_rqd_kref(pblk, rqd);
> +		pblk_read_put_line(pblk, rqd);
> 
> #ifdef CONFIG_NVM_PBLK_DEBUG
> 	atomic_long_add(rqd->nr_ppas, &pblk->sync_reads);
> --
> 2.11.0

Looks good to me

Reviewed-by: Javier González <javier@cnexlabs.com>
Matias Bjorling Aug. 20, 2018, 12:27 p.m. UTC | #2
On 08/20/2018 02:22 PM, Heiner Litz wrote:
> Consider removing the "_read", make it non-static and move the function to
> pblk-core.c as the function isn't read specific.
> 
> On Mon, Aug 20, 2018 at 1:43 PM Matias Bjørling <mb@lightnvm.io> wrote:
> 
>> The read completion path uses the put_line variable to decide whether
>> the reference on a line should be released. The function name used for
>> that is pblk_read_put_rqd_kref, which could lead one to believe that it
>> is the rqd that is releasing the reference, while it is the line
>> reference that is put.
>>
>> Rename and also split the function in two to account for either rqd or
>> single ppa callers.
>>
>> Signed-off-by: Matias Bjørling <mb@lightnvm.io>
>> ---
>>   drivers/lightnvm/pblk-read.c | 23 +++++++++++++----------
>>   1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
>> index cd2f61eed6a0..16858eaf694a 100644
>> --- a/drivers/lightnvm/pblk-read.c
>> +++ b/drivers/lightnvm/pblk-read.c
>> @@ -165,20 +165,23 @@ static void pblk_read_check_rand(struct pblk *pblk,
>> struct nvm_rq *rqd,
>>          WARN_ONCE(j != rqd->nr_ppas, "pblk: corrupted random request\n");
>>   }
>>
>> -static void pblk_read_put_rqd_kref(struct pblk *pblk, struct nvm_rq *rqd)
>> +static void __pblk_read_put_line(struct pblk *pblk, struct ppa_addr ppa)
>>   {
>> -       struct ppa_addr *ppa_list;
>> -       int i;
>> -
>> -       ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
>> -
>> -       for (i = 0; i < rqd->nr_ppas; i++) {
>> -               struct ppa_addr ppa = ppa_list[i];
>>                  struct pblk_line *line;
>>
>>                  line = &pblk->lines[pblk_ppa_to_line(ppa)];
>>                  kref_put(&line->ref, pblk_line_put_wq);
>> -       }
>> +}
>> +
>> +static void pblk_read_put_line(struct pblk *pblk, struct nvm_rq *rqd)
>> +{
>> +       struct ppa_addr *ppa_list;
>> +       int i;
>> +
>> +       ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
>> +
>> +       for (i = 0; i < rqd->nr_ppas; i++)
>> +               __pblk_read_put_line(pblk, ppa_list[i]);
>>   }
>>
>>   static void pblk_end_user_read(struct bio *bio)
>> @@ -208,7 +211,7 @@ static void __pblk_end_io_read(struct pblk *pblk,
>> struct nvm_rq *rqd,
>>                  bio_put(int_bio);
>>
>>          if (put_line)
>> -               pblk_read_put_rqd_kref(pblk, rqd);
>> +               pblk_read_put_line(pblk, rqd);
>>
>>   #ifdef CONFIG_NVM_PBLK_DEBUG
>>          atomic_long_add(rqd->nr_ppas, &pblk->sync_reads);
>> --
>> 2.11.0
>>
>>
> 

Thanks Heiner. I'll send a v2.
diff mbox series

Patch

diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index cd2f61eed6a0..16858eaf694a 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -165,20 +165,23 @@  static void pblk_read_check_rand(struct pblk *pblk, struct nvm_rq *rqd,
 	WARN_ONCE(j != rqd->nr_ppas, "pblk: corrupted random request\n");
 }
 
-static void pblk_read_put_rqd_kref(struct pblk *pblk, struct nvm_rq *rqd)
+static void __pblk_read_put_line(struct pblk *pblk, struct ppa_addr ppa)
 {
-	struct ppa_addr *ppa_list;
-	int i;
-
-	ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
-
-	for (i = 0; i < rqd->nr_ppas; i++) {
-		struct ppa_addr ppa = ppa_list[i];
 		struct pblk_line *line;
 
 		line = &pblk->lines[pblk_ppa_to_line(ppa)];
 		kref_put(&line->ref, pblk_line_put_wq);
-	}
+}
+
+static void pblk_read_put_line(struct pblk *pblk, struct nvm_rq *rqd)
+{
+	struct ppa_addr *ppa_list;
+	int i;
+
+	ppa_list = (rqd->nr_ppas > 1) ? rqd->ppa_list : &rqd->ppa_addr;
+
+	for (i = 0; i < rqd->nr_ppas; i++)
+		__pblk_read_put_line(pblk, ppa_list[i]);
 }
 
 static void pblk_end_user_read(struct bio *bio)
@@ -208,7 +211,7 @@  static void __pblk_end_io_read(struct pblk *pblk, struct nvm_rq *rqd,
 		bio_put(int_bio);
 
 	if (put_line)
-		pblk_read_put_rqd_kref(pblk, rqd);
+		pblk_read_put_line(pblk, rqd);
 
 #ifdef CONFIG_NVM_PBLK_DEBUG
 	atomic_long_add(rqd->nr_ppas, &pblk->sync_reads);