mbox series

[v5,0/5] lightnvm: Flexible metadata

Message ID 20181130114402.43793-1-igor.j.konopko@intel.com (mailing list archive)
Headers show
Series lightnvm: Flexible metadata | expand

Message

Igor Konopko Nov. 30, 2018, 11:43 a.m. UTC
This series of patches extends the way how pblk can
store L2P sector metadata. After this set of changes
any size of NVMe metadata is supported in pblk.
Also there is an support for case without NVMe metadata.

Changes v4 --> v5:
-rebase on top of ocssd/for-4.21/core

Changes v3 --> v4:
-rename nvm_alloc_dma_pool() to nvm_create_dma_pool()
-split pblk_get_meta() calls and lba setting into
two operations for better core readability
-fixing compilation with CONFIG_NVM disabled
-getting rid of unnecessary memcpy for packed metadata
on write path
-support for drives with oob size >0 and <16B in packed
metadata mode
-minor commit message updates

Changes v2 --> v3:
-Rebase on top of ocssd/for-4.21/core
-get/set_meta_lba helpers were removed
-dma reallocation was replaced with single allocation
-oob metadata size was added to pblk structure
-proper checks on pblk creation were added
 
Changes v1 --> v2:
-Revert sector meta size back to 16b for pblk
-Dma pool for larger oob meta are handled in core instead of pblk
-Pblk oob meta helpers uses __le64 as input outpu instead of u64
-Other minor fixes based on v1 patch review

Igor Konopko (5):
  lightnvm: pblk: Move lba list to partial read context
  lightnvm: pblk: Helpers for OOB metadata
  lightnvm: Flexible DMA pool entry size
  lightnvm: Disable interleaved metadata
  lightnvm: pblk: Support for packed metadata

 drivers/lightnvm/core.c          |  9 ++++--
 drivers/lightnvm/pblk-core.c     | 61 +++++++++++++++++++++++++++++++------
 drivers/lightnvm/pblk-init.c     | 44 +++++++++++++++++++++++++--
 drivers/lightnvm/pblk-map.c      | 20 +++++++-----
 drivers/lightnvm/pblk-rb.c       |  3 ++
 drivers/lightnvm/pblk-read.c     | 66 +++++++++++++++++++++++-----------------
 drivers/lightnvm/pblk-recovery.c | 25 +++++++++------
 drivers/lightnvm/pblk-sysfs.c    |  7 +++++
 drivers/lightnvm/pblk-write.c    |  9 +++---
 drivers/lightnvm/pblk.h          | 24 +++++++++++++--
 drivers/nvme/host/lightnvm.c     |  6 ++--
 include/linux/lightnvm.h         |  3 +-
 12 files changed, 209 insertions(+), 68 deletions(-)

Comments

Hans Holmberg Nov. 30, 2018, 1:03 p.m. UTC | #1
I just started a regression test on this patch set that'll run over
the weekend. I'll add a tested-by if everything checks out.

All the best,
Hans
On Fri, Nov 30, 2018 at 12:49 PM Igor Konopko <igor.j.konopko@intel.com> wrote:
>
> This series of patches extends the way how pblk can
> store L2P sector metadata. After this set of changes
> any size of NVMe metadata is supported in pblk.
> Also there is an support for case without NVMe metadata.
>
> Changes v4 --> v5:
> -rebase on top of ocssd/for-4.21/core
>
> Changes v3 --> v4:
> -rename nvm_alloc_dma_pool() to nvm_create_dma_pool()
> -split pblk_get_meta() calls and lba setting into
> two operations for better core readability
> -fixing compilation with CONFIG_NVM disabled
> -getting rid of unnecessary memcpy for packed metadata
> on write path
> -support for drives with oob size >0 and <16B in packed
> metadata mode
> -minor commit message updates
>
> Changes v2 --> v3:
> -Rebase on top of ocssd/for-4.21/core
> -get/set_meta_lba helpers were removed
> -dma reallocation was replaced with single allocation
> -oob metadata size was added to pblk structure
> -proper checks on pblk creation were added
>
> Changes v1 --> v2:
> -Revert sector meta size back to 16b for pblk
> -Dma pool for larger oob meta are handled in core instead of pblk
> -Pblk oob meta helpers uses __le64 as input outpu instead of u64
> -Other minor fixes based on v1 patch review
>
> Igor Konopko (5):
>   lightnvm: pblk: Move lba list to partial read context
>   lightnvm: pblk: Helpers for OOB metadata
>   lightnvm: Flexible DMA pool entry size
>   lightnvm: Disable interleaved metadata
>   lightnvm: pblk: Support for packed metadata
>
>  drivers/lightnvm/core.c          |  9 ++++--
>  drivers/lightnvm/pblk-core.c     | 61 +++++++++++++++++++++++++++++++------
>  drivers/lightnvm/pblk-init.c     | 44 +++++++++++++++++++++++++--
>  drivers/lightnvm/pblk-map.c      | 20 +++++++-----
>  drivers/lightnvm/pblk-rb.c       |  3 ++
>  drivers/lightnvm/pblk-read.c     | 66 +++++++++++++++++++++++-----------------
>  drivers/lightnvm/pblk-recovery.c | 25 +++++++++------
>  drivers/lightnvm/pblk-sysfs.c    |  7 +++++
>  drivers/lightnvm/pblk-write.c    |  9 +++---
>  drivers/lightnvm/pblk.h          | 24 +++++++++++++--
>  drivers/nvme/host/lightnvm.c     |  6 ++--
>  include/linux/lightnvm.h         |  3 +-
>  12 files changed, 209 insertions(+), 68 deletions(-)
>
> --
> 2.14.5
>
Matias Bjorling Nov. 30, 2018, 1:06 p.m. UTC | #2
On 11/30/2018 12:43 PM, Igor Konopko wrote:
> This series of patches extends the way how pblk can
> store L2P sector metadata. After this set of changes
> any size of NVMe metadata is supported in pblk.
> Also there is an support for case without NVMe metadata.
> 
> Changes v4 --> v5:
> -rebase on top of ocssd/for-4.21/core
> 
> Changes v3 --> v4:
> -rename nvm_alloc_dma_pool() to nvm_create_dma_pool()
> -split pblk_get_meta() calls and lba setting into
> two operations for better core readability
> -fixing compilation with CONFIG_NVM disabled
> -getting rid of unnecessary memcpy for packed metadata
> on write path
> -support for drives with oob size >0 and <16B in packed
> metadata mode
> -minor commit message updates
> 
> Changes v2 --> v3:
> -Rebase on top of ocssd/for-4.21/core
> -get/set_meta_lba helpers were removed
> -dma reallocation was replaced with single allocation
> -oob metadata size was added to pblk structure
> -proper checks on pblk creation were added
>   
> Changes v1 --> v2:
> -Revert sector meta size back to 16b for pblk
> -Dma pool for larger oob meta are handled in core instead of pblk
> -Pblk oob meta helpers uses __le64 as input outpu instead of u64
> -Other minor fixes based on v1 patch review
> 
> Igor Konopko (5):
>    lightnvm: pblk: Move lba list to partial read context
>    lightnvm: pblk: Helpers for OOB metadata
>    lightnvm: Flexible DMA pool entry size
>    lightnvm: Disable interleaved metadata
>    lightnvm: pblk: Support for packed metadata
> 
>   drivers/lightnvm/core.c          |  9 ++++--
>   drivers/lightnvm/pblk-core.c     | 61 +++++++++++++++++++++++++++++++------
>   drivers/lightnvm/pblk-init.c     | 44 +++++++++++++++++++++++++--
>   drivers/lightnvm/pblk-map.c      | 20 +++++++-----
>   drivers/lightnvm/pblk-rb.c       |  3 ++
>   drivers/lightnvm/pblk-read.c     | 66 +++++++++++++++++++++++-----------------
>   drivers/lightnvm/pblk-recovery.c | 25 +++++++++------
>   drivers/lightnvm/pblk-sysfs.c    |  7 +++++
>   drivers/lightnvm/pblk-write.c    |  9 +++---
>   drivers/lightnvm/pblk.h          | 24 +++++++++++++--
>   drivers/nvme/host/lightnvm.c     |  6 ++--
>   include/linux/lightnvm.h         |  3 +-
>   12 files changed, 209 insertions(+), 68 deletions(-)
> 

Thanks Igor. I applied the patches for 4.21. Please note that I updated 
the patch descriptions a bit. Let me know if you find anything out of place.
Hans Holmberg Dec. 3, 2018, 8:51 a.m. UTC | #3
Great! The tests(rocksdb, pblk recovery and the generic xfs suite)
completed successfully on one of our disks, so feel free to add:

Tested-by: Hans Holmberg <hans.holmberg@cnexlabs.com>

Thanks,
Hans
On Fri, Nov 30, 2018 at 2:03 PM Hans Holmberg
<hans.ml.holmberg@owltronix.com> wrote:
>
> I just started a regression test on this patch set that'll run over
> the weekend. I'll add a tested-by if everything checks out.
>
> All the best,
> Hans
> On Fri, Nov 30, 2018 at 12:49 PM Igor Konopko <igor.j.konopko@intel.com> wrote:
> >
> > This series of patches extends the way how pblk can
> > store L2P sector metadata. After this set of changes
> > any size of NVMe metadata is supported in pblk.
> > Also there is an support for case without NVMe metadata.
> >
> > Changes v4 --> v5:
> > -rebase on top of ocssd/for-4.21/core
> >
> > Changes v3 --> v4:
> > -rename nvm_alloc_dma_pool() to nvm_create_dma_pool()
> > -split pblk_get_meta() calls and lba setting into
> > two operations for better core readability
> > -fixing compilation with CONFIG_NVM disabled
> > -getting rid of unnecessary memcpy for packed metadata
> > on write path
> > -support for drives with oob size >0 and <16B in packed
> > metadata mode
> > -minor commit message updates
> >
> > Changes v2 --> v3:
> > -Rebase on top of ocssd/for-4.21/core
> > -get/set_meta_lba helpers were removed
> > -dma reallocation was replaced with single allocation
> > -oob metadata size was added to pblk structure
> > -proper checks on pblk creation were added
> >
> > Changes v1 --> v2:
> > -Revert sector meta size back to 16b for pblk
> > -Dma pool for larger oob meta are handled in core instead of pblk
> > -Pblk oob meta helpers uses __le64 as input outpu instead of u64
> > -Other minor fixes based on v1 patch review
> >
> > Igor Konopko (5):
> >   lightnvm: pblk: Move lba list to partial read context
> >   lightnvm: pblk: Helpers for OOB metadata
> >   lightnvm: Flexible DMA pool entry size
> >   lightnvm: Disable interleaved metadata
> >   lightnvm: pblk: Support for packed metadata
> >
> >  drivers/lightnvm/core.c          |  9 ++++--
> >  drivers/lightnvm/pblk-core.c     | 61 +++++++++++++++++++++++++++++++------
> >  drivers/lightnvm/pblk-init.c     | 44 +++++++++++++++++++++++++--
> >  drivers/lightnvm/pblk-map.c      | 20 +++++++-----
> >  drivers/lightnvm/pblk-rb.c       |  3 ++
> >  drivers/lightnvm/pblk-read.c     | 66 +++++++++++++++++++++++-----------------
> >  drivers/lightnvm/pblk-recovery.c | 25 +++++++++------
> >  drivers/lightnvm/pblk-sysfs.c    |  7 +++++
> >  drivers/lightnvm/pblk-write.c    |  9 +++---
> >  drivers/lightnvm/pblk.h          | 24 +++++++++++++--
> >  drivers/nvme/host/lightnvm.c     |  6 ++--
> >  include/linux/lightnvm.h         |  3 +-
> >  12 files changed, 209 insertions(+), 68 deletions(-)
> >
> > --
> > 2.14.5
> >
Hans Holmberg Dec. 4, 2018, 12:18 p.m. UTC | #4
On Mon, Dec 3, 2018 at 9:51 AM Hans Holmberg
<hans.ml.holmberg@owltronix.com> wrote:
>
> Great! The tests(rocksdb, pblk recovery and the generic xfs suite)
> completed successfully on one of our disks, so feel free to add:
>
> Tested-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
>

The tests completed fine on our hardware (which has metadata support),
but fails when I run the code on a qemu disk without metadata support.
I started to look into what goes wrong, and it seems the ppa list gets
corrupted during smeta and padding writes.

Building the kernel with CONFIG_NVM_PBLK_DEBUG, i see these errors:
[  159.735409] pblk ����: ppa: (boundary: 1) cache line: 9223372036854775807

I also got this during recovery:

[ 5384.511939] BUG: KASAN: use-after-free in pblk_get_packed_meta+0x5e/0x151
[ 5384.513036] Read of size 8 at addr ffff8881edc220e0 by task nvme/9147

There might of course be something very wrong with my qemu setup, but
it it looks more like something went wrong in the later revisions of
the patch set (i tested the one of the first versions and did not see
these issues then).

Igor: If you want it, I can share my qemu environment, more debugging
info and a couple of fixups i've done while looking into this.

Thanks,
Hans

> Thanks,
> Hans
> On Fri, Nov 30, 2018 at 2:03 PM Hans Holmberg
> <hans.ml.holmberg@owltronix.com> wrote:
> >
> > I just started a regression test on this patch set that'll run over
> > the weekend. I'll add a tested-by if everything checks out.
> >
> > All the best,
> > Hans
> > On Fri, Nov 30, 2018 at 12:49 PM Igor Konopko <igor.j.konopko@intel.com> wrote:
> > >
> > > This series of patches extends the way how pblk can
> > > store L2P sector metadata. After this set of changes
> > > any size of NVMe metadata is supported in pblk.
> > > Also there is an support for case without NVMe metadata.
> > >
> > > Changes v4 --> v5:
> > > -rebase on top of ocssd/for-4.21/core
> > >
> > > Changes v3 --> v4:
> > > -rename nvm_alloc_dma_pool() to nvm_create_dma_pool()
> > > -split pblk_get_meta() calls and lba setting into
> > > two operations for better core readability
> > > -fixing compilation with CONFIG_NVM disabled
> > > -getting rid of unnecessary memcpy for packed metadata
> > > on write path
> > > -support for drives with oob size >0 and <16B in packed
> > > metadata mode
> > > -minor commit message updates
> > >
> > > Changes v2 --> v3:
> > > -Rebase on top of ocssd/for-4.21/core
> > > -get/set_meta_lba helpers were removed
> > > -dma reallocation was replaced with single allocation
> > > -oob metadata size was added to pblk structure
> > > -proper checks on pblk creation were added
> > >
> > > Changes v1 --> v2:
> > > -Revert sector meta size back to 16b for pblk
> > > -Dma pool for larger oob meta are handled in core instead of pblk
> > > -Pblk oob meta helpers uses __le64 as input outpu instead of u64
> > > -Other minor fixes based on v1 patch review
> > >
> > > Igor Konopko (5):
> > >   lightnvm: pblk: Move lba list to partial read context
> > >   lightnvm: pblk: Helpers for OOB metadata
> > >   lightnvm: Flexible DMA pool entry size
> > >   lightnvm: Disable interleaved metadata
> > >   lightnvm: pblk: Support for packed metadata
> > >
> > >  drivers/lightnvm/core.c          |  9 ++++--
> > >  drivers/lightnvm/pblk-core.c     | 61 +++++++++++++++++++++++++++++++------
> > >  drivers/lightnvm/pblk-init.c     | 44 +++++++++++++++++++++++++--
> > >  drivers/lightnvm/pblk-map.c      | 20 +++++++-----
> > >  drivers/lightnvm/pblk-rb.c       |  3 ++
> > >  drivers/lightnvm/pblk-read.c     | 66 +++++++++++++++++++++++-----------------
> > >  drivers/lightnvm/pblk-recovery.c | 25 +++++++++------
> > >  drivers/lightnvm/pblk-sysfs.c    |  7 +++++
> > >  drivers/lightnvm/pblk-write.c    |  9 +++---
> > >  drivers/lightnvm/pblk.h          | 24 +++++++++++++--
> > >  drivers/nvme/host/lightnvm.c     |  6 ++--
> > >  include/linux/lightnvm.h         |  3 +-
> > >  12 files changed, 209 insertions(+), 68 deletions(-)
> > >
> > > --
> > > 2.14.5
> > >