Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1394476 > unrolled thread

[PATCH 1/4] lightnvm: add sync and close block I/O types

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/4] lightnvm: add sync and close block I/O types "Javier González" <jg@lightnvm.io> - 2016-05-04 17:40 +0200
    [PATCH 4/4] lightnvm: Precalculate max/min sectors per req. "Javier González" <jg@lightnvm.io> - 2016-05-04 17:40 +0200
      Re: [PATCH 4/4] lightnvm: Precalculate max/min sectors per req. Matias Bjørling <mb@lightnvm.io> - 2016-05-05 12:00 +0200
        Re: [PATCH 4/4] lightnvm: Precalculate max/min sectors per req. Javier González <javier@cnexlabs.com> - 2016-05-05 12:20 +0200
    Re: [PATCH 1/4] lightnvm: add sync and close block I/O types Matias Bjørling <mb@lightnvm.io> - 2016-05-05 11:30 +0200
      Re: [PATCH 1/4] lightnvm: add sync and close block I/O types Javier González <jg@lightnvm.io> - 2016-05-05 11:40 +0200
        Re: [PATCH 1/4] lightnvm: add sync and close block I/O types Matias Bjørling <mb@lightnvm.io> - 2016-05-05 12:10 +0200

#1394476 — [PATCH 1/4] lightnvm: add sync and close block I/O types

Within a target, I/O requests stem from different paths, which might vary
in terms of the data structures being allocated, context, etc. This
might impact how the request is treated, or how memory is freed once
the bio is completed.

Add two different types of I/Os: (i) NVM_IOTYPE_SYNC, which indicates
that the I/O is synchronous; and  (ii) NVM_IOTYPE_CLOSE_BLK, which
indicates that the I/O closes the block to which all the ppas on the
request belong to.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 include/linux/lightnvm.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 29a6890..6c02209 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -11,6 +11,8 @@ enum {
 
 	NVM_IOTYPE_NONE = 0,
 	NVM_IOTYPE_GC = 1,
+	NVM_IOTYPE_SYNC = 2,
+	NVM_IOTYPE_CLOSE_BLK = 4,
 };
 
 #define NVM_BLK_BITS (16)
-- 
2.5.0

[toc] | [next] | [standalone]

#1394480 — [PATCH 4/4] lightnvm: Precalculate max/min sectors per req.

Add two precalculated values to nvm_rq: (i) maximum number of sectors
per general request; and (ii) minimum number of sectors per write
request.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/lightnvm/core.c  | 3 +++
 include/linux/lightnvm.h | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 4cd9803..85682d91 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -573,6 +573,9 @@ static int nvm_core_init(struct nvm_dev *dev)
 
 	dev->plane_mode = NVM_PLANE_SINGLE;
 	dev->max_rq_size = dev->ops->max_phys_sect * dev->sec_size;
+	dev->max_sec_rq = dev->ops->max_phys_sect;
+	/* assume max_phys_sect % dev->min_write_pgs == 0 */
+	dev->min_sec_w_rq = dev->sec_per_pl * (dev->sec_size / PAGE_SIZE);
 
 	if (grp->mpos & 0x020202)
 		dev->plane_mode = NVM_PLANE_DOUBLE;
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 67e72f5..c2dfd0c 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -351,7 +351,9 @@ struct nvm_dev {
 	/* Calculated/Cached values. These do not reflect the actual usable
 	 * blocks at run-time.
 	 */
-	int max_rq_size;
+	int max_rq_size; /* maximum size of a single request */
+	int max_sec_rq; /* maximum amount of sectors that fit in one req. */
+	int min_sec_w_rq; /* minimum amount of sectors required on write req. */
 	int plane_mode; /* drive device in single, double or quad mode */
 
 	int sec_per_pl; /* all sectors across planes */
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1395003 — Re: [PATCH 4/4] lightnvm: Precalculate max/min sectors per req.

On 05/04/2016 05:31 PM, Javier González wrote:
> Add two precalculated values to nvm_rq: (i) maximum number of sectors
> per general request; and (ii) minimum number of sectors per write
> request.
>
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>   drivers/lightnvm/core.c  | 3 +++
>   include/linux/lightnvm.h | 4 +++-
>   2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 4cd9803..85682d91 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -573,6 +573,9 @@ static int nvm_core_init(struct nvm_dev *dev)
>
>   	dev->plane_mode = NVM_PLANE_SINGLE;
>   	dev->max_rq_size = dev->ops->max_phys_sect * dev->sec_size;
> +	dev->max_sec_rq = dev->ops->max_phys_sect;
> +	/* assume max_phys_sect % dev->min_write_pgs == 0 */
> +	dev->min_sec_w_rq = dev->sec_per_pl * (dev->sec_size / PAGE_SIZE);
>
>   	if (grp->mpos & 0x020202)
>   		dev->plane_mode = NVM_PLANE_DOUBLE;
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 67e72f5..c2dfd0c 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -351,7 +351,9 @@ struct nvm_dev {
>   	/* Calculated/Cached values. These do not reflect the actual usable
>   	 * blocks at run-time.
>   	 */
> -	int max_rq_size;
> +	int max_rq_size; /* maximum size of a single request */
> +	int max_sec_rq; /* maximum amount of sectors that fit in one req. */
> +	int min_sec_w_rq; /* minimum amount of sectors required on write req. */
>   	int plane_mode; /* drive device in single, double or quad mode */
>
>   	int sec_per_pl; /* all sectors across planes */
>

I think this is best kept within pblk for now. The min_sec_w_rq is not 
enough to describe the pages to be written. It must also follow a 
specific order, which is not communicated only with minimum page writes.

[toc] | [prev] | [next] | [standalone]

#1395020 — Re: [PATCH 4/4] lightnvm: Precalculate max/min sectors per req.

> On 05 May 2016, at 11:54, Matias Bjørling <mb@lightnvm.io> wrote:
> 
>> On 05/04/2016 05:31 PM, Javier González wrote:
>> Add two precalculated values to nvm_rq: (i) maximum number of sectors
>> per general request; and (ii) minimum number of sectors per write
>> request.
>> 
>> Signed-off-by: Javier González <javier@cnexlabs.com>
>> ---
>>  drivers/lightnvm/core.c  | 3 +++
>>  include/linux/lightnvm.h | 4 +++-
>>  2 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index 4cd9803..85682d91 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -573,6 +573,9 @@ static int nvm_core_init(struct nvm_dev *dev)
>> 
>>      dev->plane_mode = NVM_PLANE_SINGLE;
>>      dev->max_rq_size = dev->ops->max_phys_sect * dev->sec_size;
>> +    dev->max_sec_rq = dev->ops->max_phys_sect;
>> +    /* assume max_phys_sect % dev->min_write_pgs == 0 */
>> +    dev->min_sec_w_rq = dev->sec_per_pl * (dev->sec_size / PAGE_SIZE);
>> 
>>      if (grp->mpos & 0x020202)
>>          dev->plane_mode = NVM_PLANE_DOUBLE;
>> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
>> index 67e72f5..c2dfd0c 100644
>> --- a/include/linux/lightnvm.h
>> +++ b/include/linux/lightnvm.h
>> @@ -351,7 +351,9 @@ struct nvm_dev {
>>      /* Calculated/Cached values. These do not reflect the actual usable
>>       * blocks at run-time.
>>       */
>> -    int max_rq_size;
>> +    int max_rq_size; /* maximum size of a single request */
>> +    int max_sec_rq; /* maximum amount of sectors that fit in one req. */
>> +    int min_sec_w_rq; /* minimum amount of sectors required on write req. */
>>      int plane_mode; /* drive device in single, double or quad mode */
>> 
>>      int sec_per_pl; /* all sectors across planes */
> 
> I think this is best kept within pblk for now. The min_sec_w_rq is not enough to describe the pages to be written. It must also follow a specific order, which is not communicated only with minimum page writes.

Yes, you're right. Let us wait until we can communicate everything to the target. 

Thanks!
Javier

[toc] | [prev] | [next] | [standalone]

#1394954

On 05/04/2016 05:31 PM, Javier González wrote:
> Within a target, I/O requests stem from different paths, which might vary
> in terms of the data structures being allocated, context, etc. This
> might impact how the request is treated, or how memory is freed once
> the bio is completed.
>
> Add two different types of I/Os: (i) NVM_IOTYPE_SYNC, which indicates
> that the I/O is synchronous; and  (ii) NVM_IOTYPE_CLOSE_BLK, which
> indicates that the I/O closes the block to which all the ppas on the
> request belong to.
>
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>   include/linux/lightnvm.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 29a6890..6c02209 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -11,6 +11,8 @@ enum {
>
>   	NVM_IOTYPE_NONE = 0,
>   	NVM_IOTYPE_GC = 1,
> +	NVM_IOTYPE_SYNC = 2,
> +	NVM_IOTYPE_CLOSE_BLK = 4,
>   };
>
>   #define NVM_BLK_BITS (16)
>

The sync should not be necessary when the read path is implemented using 
bio_clone. Similarly for NVM_IOTYPE_CLOSE_BLK. The write completion can 
be handled in the bio completion path.

[toc] | [prev] | [next] | [standalone]

#1394971

[Multipart message — attachments visible in raw view] — view raw

> On 05 May 2016, at 11:21, Matias Bjørling <mb@lightnvm.io> wrote:
> 
> On 05/04/2016 05:31 PM, Javier González wrote:
>> Within a target, I/O requests stem from different paths, which might vary
>> in terms of the data structures being allocated, context, etc. This
>> might impact how the request is treated, or how memory is freed once
>> the bio is completed.
>> 
>> Add two different types of I/Os: (i) NVM_IOTYPE_SYNC, which indicates
>> that the I/O is synchronous; and  (ii) NVM_IOTYPE_CLOSE_BLK, which
>> indicates that the I/O closes the block to which all the ppas on the
>> request belong to.
>> 
>> Signed-off-by: Javier González <javier@cnexlabs.com>
>> ---
>>  include/linux/lightnvm.h | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
>> index 29a6890..6c02209 100644
>> --- a/include/linux/lightnvm.h
>> +++ b/include/linux/lightnvm.h
>> @@ -11,6 +11,8 @@ enum {
>> 
>>  	NVM_IOTYPE_NONE = 0,
>>  	NVM_IOTYPE_GC = 1,
>> +	NVM_IOTYPE_SYNC = 2,
>> +	NVM_IOTYPE_CLOSE_BLK = 4,
>>  };
>> 
>>  #define NVM_BLK_BITS (16)
> 
> The sync should not be necessary when the read path is implemented
> using bio_clone. Similarly for NVM_IOTYPE_CLOSE_BLK. The write
> completion can be handled in the bio completion path.

We need to know where the request comes from; we cannot do it just from
having the bio. This is because we allocate different structures
depending on the type of bio we send. It is not only which bio->end_io
function we have, but which memory needs to be released. Sync is
necessary for the read path when we have a partial bio (data both on
write buffer and disk) that we need to fill up. Also for GC.. In this
case, the bio is to be freed differently. In the case of close the case
is similarly; we do not free memory on the end_io path, but on the caller.

You can see how these flags are used on pblk. Maybe there is a better
way of doing it that I could not see...

Javier

[toc] | [prev] | [next] | [standalone]

#1395013

On 05/05/2016 11:38 AM, Javier González wrote:
>
>> On 05 May 2016, at 11:21, Matias Bjørling <mb@lightnvm.io> wrote:
>>
>> On 05/04/2016 05:31 PM, Javier González wrote:
>>> Within a target, I/O requests stem from different paths, which might vary
>>> in terms of the data structures being allocated, context, etc. This
>>> might impact how the request is treated, or how memory is freed once
>>> the bio is completed.
>>>
>>> Add two different types of I/Os: (i) NVM_IOTYPE_SYNC, which indicates
>>> that the I/O is synchronous; and  (ii) NVM_IOTYPE_CLOSE_BLK, which
>>> indicates that the I/O closes the block to which all the ppas on the
>>> request belong to.
>>>
>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>> ---
>>>   include/linux/lightnvm.h | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
>>> index 29a6890..6c02209 100644
>>> --- a/include/linux/lightnvm.h
>>> +++ b/include/linux/lightnvm.h
>>> @@ -11,6 +11,8 @@ enum {
>>>
>>>   	NVM_IOTYPE_NONE = 0,
>>>   	NVM_IOTYPE_GC = 1,
>>> +	NVM_IOTYPE_SYNC = 2,
>>> +	NVM_IOTYPE_CLOSE_BLK = 4,
>>>   };
>>>
>>>   #define NVM_BLK_BITS (16)
>>
>> The sync should not be necessary when the read path is implemented
>> using bio_clone. Similarly for NVM_IOTYPE_CLOSE_BLK. The write
>> completion can be handled in the bio completion path.
>
> We need to know where the request comes from; we cannot do it just from
> having the bio. This is because we allocate different structures
> depending on the type of bio we send. It is not only which bio->end_io
> function we have, but which memory needs to be released. Sync is
> necessary for the read path when we have a partial bio (data both on
> write buffer and disk) that we need to fill up. Also for GC.. In this
> case, the bio is to be freed differently. In the case of close the case
> is similarly; we do not free memory on the end_io path, but on the caller.
>
> You can see how these flags are used on pblk. Maybe there is a better
> way of doing it that I could not see...
>

Use the bio completion path for both. For bio completion, free the data 
that is specific to the pblk flow and let the nvm_rq->end_io() 
completion path clean up the nvm_rq specific data. That will clean up 
the completion paths.

> Javier
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web