[PATCH v5] lightnvn: pblk

[PATCH v5] lightnvn: pblk

[Javier González]

Changes since v4:
* Rebase on top of Matias' for-4.12/core
* Fix type implicit conversions reported by sparse (reported by Bart Van
  Assche)
* Make error and debug statistics long atomic variables.

Changes since v3:
* Apply Bart's feedback [1]
* Implement dynamic L2P optimizations for > 32-bit physical media
  geometry (from Matias Bjørling)
* Fix memory leak on GC (Reported by Simon A. F. Lund)
* 8064 is a perfectly round number of lines :)

[1] https://lkml.org/lkml/2017/4/8/172

Changes since v2:
* Rebase on top of Matias' for-4.12/core
* Implement L2P scan recovery to recover L2P table in case of power
failure.
* Re-design disk format to be more flexible in future versions (from
 Matias Bjørling)
* Implement per-instance uuid to allow correct recovery without
 forcing line erases (from Matias Bjørling)
* Re-design GC threading to have several GC readers and a single
  writer that places data on the write buffer. This allows to maximize
  the GC write buffer budget without having unnecessary GC writers
  competing for the write buffer lock.
* Simplify sysfs interface.
* Refactoring and several code improvements (together with Matias
  Bjørling)

Changes since v1:
* Rebase on top of Matias' for-4.12/core
* Move from per-LUN block allocation to a line model. This means that a
  whole lines across all LUNs is allocated at a time. Data is still
  stripped in a round-robin fashion at a page granurality.
* Implement new disk format scheme, where metadata is stored per line
  instead of per LUN. This allows for space optimizations.
* Improvements on GC workqueue management and victim selection.
* Implement sysfs interface to query pblk's operation and statistics.
* Implement a user - GC I/O rate-limiter
* Various bug fixes

Javier González (1):
  lightnvm: physical block device (pblk) target

 Documentation/lightnvm/pblk.txt  |   21 +
 drivers/lightnvm/Kconfig         |    9 +
 drivers/lightnvm/Makefile        |    5 +
 drivers/lightnvm/pblk-cache.c    |  114 +++
 drivers/lightnvm/pblk-core.c     | 1655 ++++++++++++++++++++++++++++++++++++++
 drivers/lightnvm/pblk-gc.c       |  555 +++++++++++++
 drivers/lightnvm/pblk-init.c     |  949 ++++++++++++++++++++++
 drivers/lightnvm/pblk-map.c      |  139 ++++
 drivers/lightnvm/pblk-rb.c       |  852 ++++++++++++++++++++
 drivers/lightnvm/pblk-read.c     |  529 ++++++++++++
 drivers/lightnvm/pblk-recovery.c |  998 +++++++++++++++++++++++
 drivers/lightnvm/pblk-rl.c       |  182 +++++
 drivers/lightnvm/pblk-sysfs.c    |  507 ++++++++++++
 drivers/lightnvm/pblk-write.c    |  411 ++++++++++
 drivers/lightnvm/pblk.h          | 1121 ++++++++++++++++++++++++++
 15 files changed, 8047 insertions(+)
 create mode 100644 Documentation/lightnvm/pblk.txt
 create mode 100644 drivers/lightnvm/pblk-cache.c
 create mode 100644 drivers/lightnvm/pblk-core.c
 create mode 100644 drivers/lightnvm/pblk-gc.c
 create mode 100644 drivers/lightnvm/pblk-init.c
 create mode 100644 drivers/lightnvm/pblk-map.c
 create mode 100644 drivers/lightnvm/pblk-rb.c
 create mode 100644 drivers/lightnvm/pblk-read.c
 create mode 100644 drivers/lightnvm/pblk-recovery.c
 create mode 100644 drivers/lightnvm/pblk-rl.c
 create mode 100644 drivers/lightnvm/pblk-sysfs.c
 create mode 100644 drivers/lightnvm/pblk-write.c
 create mode 100644 drivers/lightnvm/pblk.h

-- 
2.7.4

Re: [PATCH v5] lightnvm: physical block device (pblk) target

[Matias Bjørling]

On 04/11/2017 04:31 PM, Javier González wrote:
> This patch introduces pblk, a host-side translation layer for
> Open-Channel SSDs to expose them like block devices. The translation
> layer allows data placement decisions, and I/O scheduling to be
> managed by the host, enabling users to optimize the SSD for their
> specific workloads.
>
> An open-channel SSD has a set of LUNs (parallel units) and a
> collection of blocks. Each block can be read in any order, but
> writes must be sequential. Writes may also fail, and if a block
> requires it, must also be reset before new writes can be
> applied.
>
> To manage the constraints, pblk maintains a logical to
> physical address (L2P) table,  write cache, garbage
> collection logic, recovery scheme, and logic to rate-limit
> user I/Os versus garbage collection I/Os.
>
> The L2P table is fully-associative and manages sectors at a
> 4KB granularity. Pblk stores the L2P table in two places, in
> the out-of-band area of the media and on the last page of a
> line. In the cause of a power failure, pblk will perform a
> scan to recover the L2P table.
>
> The user data is organized into lines. A line is data
> striped across blocks and LUNs. The lines enable the host to
> reduce the amount of metadata to maintain besides the user
> data and makes it easier to implement RAID or erasure coding
> in the future.
>
> pblk implements multi-tenant support and can be instantiated
> multiple times on the same drive. Each instance owns a
> portion of the SSD - both regarding I/O bandwidth and
> capacity - providing I/O isolation for each case.
>
> Finally, pblk also exposes a sysfs interface that allows
> user-space to peek into the internals of pblk. The interface
> is available at /dev/block/*/pblk/ where * is the block
> device name exposed.
>
> This work also contains contributions from:
>   Matias Bjørling <matias@cnexlabs.com>
>   Simon A. F. Lund <slund@cnexlabs.com>
>   Young Tack Jin <youngtack.jin@gmail.com>
>   Huaicheng Li <huaicheng@cs.uchicago.edu>
>
> Signed-off-by: Javier González <javier@cnexlabs.com>

Awesome. Applied for 4.12.

Re: [PATCH v5] lightnvn: pblk

[Bart Van Assche]

On Tue, 2017-04-11 at 16:31 +0200, Javier Gonz=E1lez wrote:
> Changes since v4:
> * Rebase on top of Matias' for-4.12/core
> * Fix type implicit conversions reported by sparse (reported by Bart Van
>   Assche)
> * Make error and debug statistics long atomic variables.

Hello Javier,

Thanks for the quick respin. But have you already had a look at the
diagnostics reported by smatch? Smatch reports e.g.

drivers/lightnvm/pblk-rb.c:783: pblk_rb_tear_down_check() error: we previou=
sly assumed 'rb->entries' could be null (see line 779)

on the following code:

	if (rb->entries)
		goto out;

	for (i =3D 0; i < rb->nr_entries; i++) {
		entry =3D &rb->entries[i];

		if (entry->data)
			goto out;
	}

Is that "if (rb->entries)" check correct or should that perhaps been
"if (!rb->entries)"? Smatch is available at http://repo.or.cz/w/smatch.git.

Bart.=

Re: [PATCH v5] lightnvn: pblk

[Javier González]

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

Hi Bart,

> On 11 Apr 2017, at 17.19, Bart Van Assche <Bart.VanAssche@sandisk.com> wrote:
> 
> On Tue, 2017-04-11 at 16:31 +0200, Javier González wrote:
>> Changes since v4:
>> * Rebase on top of Matias' for-4.12/core
>> * Fix type implicit conversions reported by sparse (reported by Bart Van
>>  Assche)
>> * Make error and debug statistics long atomic variables.
> 
> Hello Javier,
> 
> Thanks for the quick respin. But have you already had a look at the
> diagnostics reported by smatch? Smatch reports e.g.
> 
> drivers/lightnvm/pblk-rb.c:783: pblk_rb_tear_down_check() error: we previously assumed 'rb->entries' could be null (see line 779)
> 
> on the following code:
> 
> 	if (rb->entries)
> 		goto out;
> 
> 	for (i = 0; i < rb->nr_entries; i++) {
> 		entry = &rb->entries[i];
> 
> 		if (entry->data)
> 			goto out;
> 	}
> 
> Is that "if (rb->entries)" check correct or should that perhaps been
> "if (!rb->entries)"? Smatch is available at http://repo.or.cz/w/smatch.git.

I have run smatch over the code (did not know the tool, so thanks!).
This particular error has been fixed on v5. The only standing warning
relates to a semaphore on pblk-map that is taken on the erase path. This
is a false positive; it is intended that the semaphore lock is taken
here and then released on the completion path. Sparse and coccicheck
have been also been used on v5, but please point out to any other
tools/concerns you may have.

> Bart.

Thanks,

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v5] lightnvn: pblk

[Bart Van Assche]

On Wed, 2017-04-12 at 00:13 +0200, Javier Gonz=E1lez wrote:
> please point out to any other tools/concerns you may have.

Hello Javier,

Do you already have an account at https://scan.coverity.com/? Any Linux
kernel developer can get an account for free. A full Coverity scan of
Linus' tree is available at https://scan.coverity.com/projects/linux.

Bart.=

Re: [PATCH v5] lightnvn: pblk

[Javier González]

[-- Attachment #1: Type: text/plain, Size: 572 bytes --]

> On 12 Apr 2017, at 00.23, Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> 
> On Wed, 2017-04-12 at 00:13 +0200, Javier González wrote:
>> please point out to any other tools/concerns you may have.
> 
> Hello Javier,
> 
> Do you already have an account at https://scan.coverity.com/? Any Linux
> kernel developer can get an account for free. A full Coverity scan of
> Linus' tree is available at https://scan.coverity.com/projects/linux.

Hi Bart,

No I did not. Thanks for the invite. I just created an account now;
waiting for approval.

Javier

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 801 bytes --]