All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@sandisk.com>
To: "jg@lightnvm.io" <jg@lightnvm.io>
Cc: "mb@lightnvm.io" <mb@lightnvm.io>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [PATCH v3] lightnvm: physical block device (pblk) target
Date: Mon, 10 Apr 2017 15:55:43 +0000	[thread overview]
Message-ID: <1491839741.4199.7.camel@sandisk.com> (raw)
In-Reply-To: <E42A544D-D09C-4DDB-800A-9FCED4CBBD28@lightnvm.io>

On Sun, 2017-04-09 at 11:15 +0200, Javier Gonz=E1lez wrote:
> On 8 Apr 2017, at 22.56, Bart Van Assche <bart.vanassche@sandisk.com> wro=
te:
> > On 04/07/17 11:50, Javier Gonz=E1lez wrote:
> struct ppa_addr, which is the physical address format is not affected,
> but pblk's internal L2P address representation (pblk_addr) is. You can
> see that the type either represents struct ppa_addr or ppa_addr_32. How
> would you define a type that can either be u64 or u32 with different bit
> offsets at run-time? Note that address conversions to this type is in
> the fast path and this format allows us to only use bit shifts.

Selecting the appropriate representation at run-time would require to pass
pblk_addr by reference instead of by value to any function that expects a
pblk_addr. It would also require to have two versions of every data structu=
re
that depends on pblk_addr and to use casts to convert to the appropritate
version. However, this approach is probably only worth to be implemented if
it doesn't introduce too much additional complexity.

> > > +#ifdef CONFIG_NVM_DEBUG
> > > +	atomic_add(nr_entries, &pblk->inflight_writes);
> > > +	atomic_add(nr_entries, &pblk->req_writes);
> > > +#endif
> >=20
> > Has it been considered to use the "static key" feature such that
> > consistency checks can be enabled at run-time instead of having to
> > rebuild the kernel to enable CONFIG_NVM_DEBUG?
>=20
> I haven't considered it. I'll look into it. I would like to have this
> counters and the corresponding sysfs entry only available on debug mode
> since it allows us to have a good picture of the FTL state.

If there are sysfs entries that depend on CONFIG_NVM_DEBUG then the static
key mechanism is probably not a good alternative for CONFIG_NVM_DEBUG.

> > Has it been considered to add support for keeping a subset of the L2P
> > translation table in memory instead of keeping it in memory in its enti=
rety?
>=20
> Yes. L2P caching is on our roadmap and will be included in the future.

That's great. This will also help with reducing the time between discovery =
of
a lightnvm device and the time at which I/O can start since the L2P table m=
ust
be available before I/O can start.

Bart.=

WARNING: multiple messages have this Message-ID (diff)
From: Bart Van Assche <Bart.VanAssche@sandisk.com>
To: "jg@lightnvm.io" <jg@lightnvm.io>
Cc: "mb@lightnvm.io" <mb@lightnvm.io>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [PATCH v3] lightnvm: physical block device (pblk) target
Date: Mon, 10 Apr 2017 15:55:43 +0000	[thread overview]
Message-ID: <1491839741.4199.7.camel@sandisk.com> (raw)
In-Reply-To: <E42A544D-D09C-4DDB-800A-9FCED4CBBD28@lightnvm.io>

On Sun, 2017-04-09 at 11:15 +0200, Javier González wrote:
> On 8 Apr 2017, at 22.56, Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> > On 04/07/17 11:50, Javier González wrote:
> struct ppa_addr, which is the physical address format is not affected,
> but pblk's internal L2P address representation (pblk_addr) is. You can
> see that the type either represents struct ppa_addr or ppa_addr_32. How
> would you define a type that can either be u64 or u32 with different bit
> offsets at run-time? Note that address conversions to this type is in
> the fast path and this format allows us to only use bit shifts.

Selecting the appropriate representation at run-time would require to pass
pblk_addr by reference instead of by value to any function that expects a
pblk_addr. It would also require to have two versions of every data structure
that depends on pblk_addr and to use casts to convert to the appropritate
version. However, this approach is probably only worth to be implemented if
it doesn't introduce too much additional complexity.

> > > +#ifdef CONFIG_NVM_DEBUG
> > > +	atomic_add(nr_entries, &pblk->inflight_writes);
> > > +	atomic_add(nr_entries, &pblk->req_writes);
> > > +#endif
> > 
> > Has it been considered to use the "static key" feature such that
> > consistency checks can be enabled at run-time instead of having to
> > rebuild the kernel to enable CONFIG_NVM_DEBUG?
> 
> I haven't considered it. I'll look into it. I would like to have this
> counters and the corresponding sysfs entry only available on debug mode
> since it allows us to have a good picture of the FTL state.

If there are sysfs entries that depend on CONFIG_NVM_DEBUG then the static
key mechanism is probably not a good alternative for CONFIG_NVM_DEBUG.

> > Has it been considered to add support for keeping a subset of the L2P
> > translation table in memory instead of keeping it in memory in its entirety?
> 
> Yes. L2P caching is on our roadmap and will be included in the future.

That's great. This will also help with reducing the time between discovery of
a lightnvm device and the time at which I/O can start since the L2P table must
be available before I/O can start.

Bart.

  reply	other threads:[~2017-04-10 15:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-07 18:50 [PATCH v3] lightnvm: pblk Javier González
2017-04-07 18:50 ` [PATCH v3] lightnvm: physical block device (pblk) target Javier González
2017-04-08 20:56   ` Bart Van Assche
2017-04-08 20:56     ` Bart Van Assche
2017-04-09  9:15     ` Javier González
2017-04-10 15:55       ` Bart Van Assche [this message]
2017-04-10 15:55         ` Bart Van Assche
2017-04-10 17:47         ` Matias Bjørling
2017-04-10 17:47           ` Matias Bjørling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1491839741.4199.7.camel@sandisk.com \
    --to=bart.vanassche@sandisk.com \
    --cc=jg@lightnvm.io \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mb@lightnvm.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.