On 1 Oct 2017, at 15.23, Rakesh Pandit <[email protected]> wrote:

We already pass the structure pointer so no need to pass the member.

Signed-off-by: Rakesh Pandit <[email protected]>
---
drivers/lightnvm/pblk-rb.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
index 05e6b2e..920ffac 100644
--- a/drivers/lightnvm/pblk-rb.c
+++ b/drivers/lightnvm/pblk-rb.c
@@ -201,9 +201,9 @@ unsigned int pblk_rb_read_commit(struct pblk_rb *rb, unsigned int nr_entries)
return subm;
}

-static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
- unsigned int to_update)
+static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update) {
+ unsigned int l2p_update = rb->l2p_update;
struct pblk *pblk = container_of(rb, struct pblk, rwb);
struct pblk_line *line;
struct pblk_rb_entry *entry;
@@ -213,7 +213,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
int flags;

for (i = 0; i < to_update; i++) {
- entry = &rb->entries[*l2p_upd];
+ entry = &rb->entries[l2p_update];
w_ctx = &entry->w_ctx;

flags = READ_ONCE(entry->w_ctx.flags);
@@ -230,7 +230,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
line = &pblk->lines[pblk_tgt_ppa_to_line(w_ctx->ppa)];
kref_put(&line->ref, pblk_line_put);
clean_wctx(w_ctx);
- *l2p_upd = (*l2p_upd + 1) & (rb->nr_entries - 1);
+ rb->l2p_update = (l2p_update + 1) & (rb->nr_entries - 1);
}

pblk_rl_out(&pblk->rl, user_io, gc_io);
@@ -258,7 +258,7 @@ static int pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int nr_entries,

count = nr_entries - space;
/* l2p_update used exclusively under rb->w_lock */
- ret = __pblk_rb_update_l2p(rb, &rb->l2p_update, count);
+ ret = __pblk_rb_update_l2p(rb, count);

out:
return ret;
@@ -280,7 +280,7 @@ void pblk_rb_sync_l2p(struct pblk_rb *rb)
sync = smp_load_acquire(&rb->sync);

to_update = pblk_rb_ring_count(sync, rb->l2p_update, rb->nr_entries);
- __pblk_rb_update_l2p(rb, &rb->l2p_update, to_update);
+ __pblk_rb_update_l2p(rb, to_update);

spin_unlock(&rb->w_lock);
}
--
2.7.4

This in inherited from the time when the buffer pointers where kept in a separate structure. While they are kept in struct pblk_rb, this is a
good cleanup.

Reviewed-by: Javier González <[email protected]>


-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEm1mT7zen+vs9+T8kYx8FO3WZGMoFAlnSI+kACgkQYx8FO3WZ GMqytBAA2wgGKIvqHJbkZhbkVYsvqk0ihU0XEJsuZDVAEwcqiR3JhvVvp8xKhqgu 1EevpF88H5n9EzZo5ibIMvWzyEIgtBXRFjlsqOpHqRop5SXT69C7hUamDaOu/e1n mEpyFZPecXsz4+OyYPLOZgZlPmjxY7MAJps3UAHpoTQODyiqM3WEwXFa/IoEQyLj GXZm5FZzCf7TEp2HP7iWcGsxKXUfpIp9P4lwqeTL3kOJk/SUgvIqDMYRMINVFuyi m/DhqP7tjH+uy+/kmcBLfi79KoNR+9sLDYBmoK84Dxh2Pg48TZyeJNEjNhyL+ytN mXZwoGKELrR+CqPVTXdgCzMx7IKHs7gsn96NOSfI5YV4op3mnq8Z80kuDjmzN15G K3bLVSNqj6bOTIe/3wgHxYmQy4dFp7nGHs8fWVrqyzVYpP3/1OC5sZknWIPca9I6 MEP70ng/md7Hz9MNMF6sQ++epQNDpxWa15DA7XxakKsUaMaBWHmkQjICPLxWwRaW 0nkY5mwy3zXiBGwU7iuOlQ72os2LqVmmypuklqNqw3+WGkd+ulJB+Sdk6kXjYPA5 GNGJT+dk1juFAi3L6zQAGs5MSm3LkGKw6Xsa4kEvCB7s9w1539A5vPsWy80FhRD4 hJf9vhFbErQVa55mdIX35oA9yHxBIpxJkMMAvG9uqEysSIX3uZg=
=8nCb
-----END PGP SIGNATURE-----

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On 2 Oct 2017, at 13.32, Javier González <[email protected]> wrote:

On 1 Oct 2017, at 15.23, Rakesh Pandit <[email protected]> wrote:

We already pass the structure pointer so no need to pass the member.

Signed-off-by: Rakesh Pandit <[email protected]>
---
drivers/lightnvm/pblk-rb.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
index 05e6b2e..920ffac 100644
--- a/drivers/lightnvm/pblk-rb.c
+++ b/drivers/lightnvm/pblk-rb.c
@@ -201,9 +201,9 @@ unsigned int pblk_rb_read_commit(struct pblk_rb *rb, unsigned int nr_entries)
return subm;
}

-static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd, >> - unsigned int to_update)
+static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update) >> {
+ unsigned int l2p_update = rb->l2p_update;
struct pblk *pblk = container_of(rb, struct pblk, rwb);
struct pblk_line *line;
struct pblk_rb_entry *entry;
@@ -213,7 +213,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
int flags;

for (i = 0; i < to_update; i++) {
- entry = &rb->entries[*l2p_upd];
+ entry = &rb->entries[l2p_update];
w_ctx = &entry->w_ctx;

flags = READ_ONCE(entry->w_ctx.flags);
@@ -230,7 +230,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
line = &pblk->lines[pblk_tgt_ppa_to_line(w_ctx->ppa)];
kref_put(&line->ref, pblk_line_put);
clean_wctx(w_ctx);
- *l2p_upd = (*l2p_upd + 1) & (rb->nr_entries - 1);
+ rb->l2p_update = (l2p_update + 1) & (rb->nr_entries - 1);

This is wrong. It should be rb->l2p_update when doing +1, otherwise you
are not using the updated l2p_update value. The result is the pipeline
stalling as the l2p pointer is left behind.

I'll fix it when picking it up.

Please test the patches before submitting.

Javier.

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEm1mT7zen+vs9+T8kYx8FO3WZGMoFAlnSWGkACgkQYx8FO3WZ GMqnAA//ZQGhx0evlyWMTVAaAfr916IjO1Ox2RMxYamvwiQPplEETlmbOJD61dQ2 8+oss8PypeQjxhPz2fLvMUPJmzxwH12X+DOajNBbmD9sl1NEMPgIDi/w/Ubo6YNo Qjshnxt7oCzpwR1xVEVFD0wmPzpp6jNWES1GfbVCPUbBSC7ZG6ptlkcS2TgUGlqk dIu36Jg8/jdV5BwClelgHyJqzGpfUc8frYybtx4MjzVr/x3aBA8uHu7dOQja6Nfl njnHCj+cWnJe5MogRYWwT/qn7o/tRxzrcz6CF+rWqEcl9XLY0YXVoqQ0XK9QU55D 3kRDOEtM0r9LlXG09Y1+ljNA07q7eqBqRFs9ZhIqB4Ltz3cw9X5E4rz4324AA1KI l3KfW7Q60E9Yk/m0XJKMbKL9ts2h/2dmjMBCh3x7fTcWVwBP0M+aVnuCd6phN7Dq TPD6eeAmINqK+mJyQlQMXEee3p+IjutPMRii6Vyh36XVnz0auOcl9nOyck3l77rP 8Gf/v9TZ/I41MgN2J+bZAfxHDnsPthOFnZJn9pg+Fhkma+A+WpTtCAEKLLWmc/El jymiQDL4AkD0mkdHK6chu7I1JB0/iGM2+6I6ta/ycL2xC9+coQvdTmh9x7DGyxoG vDEq+c1jeflWsuv57j7PQsM6zxqwFmNbdh2NFrywm7uTn+waF78=
=07bw
-----END PGP SIGNATURE-----

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On Mon, Oct 02, 2017 at 05:16:57PM +0200, Javier Gonz�lez wrote:

On 2 Oct 2017, at 13.32, Javier Gonz�lez <[email protected]> wrote:

On 1 Oct 2017, at 15.23, Rakesh Pandit <[email protected]> wrote:

[..]

-static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
- unsigned int to_update)
+static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update)
{
+ unsigned int l2p_update = rb->l2p_update;
struct pblk *pblk = container_of(rb, struct pblk, rwb);
struct pblk_line *line;
struct pblk_rb_entry *entry;
@@ -213,7 +213,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
int flags;

for (i = 0; i < to_update; i++) {
- entry = &rb->entries[*l2p_upd];
+ entry = &rb->entries[l2p_update];
w_ctx = &entry->w_ctx;

flags = READ_ONCE(entry->w_ctx.flags);
@@ -230,7 +230,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
line = &pblk->lines[pblk_tgt_ppa_to_line(w_ctx->ppa)];
kref_put(&line->ref, pblk_line_put);
clean_wctx(w_ctx);
- *l2p_upd = (*l2p_upd + 1) & (rb->nr_entries - 1);
+ rb->l2p_update = (l2p_update + 1) & (rb->nr_entries - 1);

This is wrong. It should be rb->l2p_update when doing +1, otherwise you
are not using the updated l2p_update value. The result is the pipeline stalling as the l2p pointer is left behind.

I'll fix it when picking it up.

Please test the patches before submitting.

Thanks for fixing it. I would be more careful. And will make sure
everything goes through testing before.

Regards,

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)