From 8c2cff02b6e1641c42bb3021e79c3d2cb12ea605 Mon Sep 17 00:00:00 2001 From: Mateusz Kozlowski Date: Thu, 6 Jun 2019 15:24:05 +0200 Subject: [PATCH] lib/ftl: Fix ppa pack function Address translation wasn't correct for >32 bit length packed address. This commit fixes the issue and adds a corresponding unit test. This patch fixes issue #774: https://github.com/spdk/spdk/issues/774 Signed-off-by: Mateusz Kozlowski Change-Id: Idce67c47f2a9888f9e2ae2eadaf71ccc34e5c260 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/457114 Tested-by: SPDK CI Jenkins Reviewed-by: Wojciech Malikowski Reviewed-by: Konrad Sztyber Reviewed-by: Darek Stojaczyk Reviewed-by: Ben Walker --- lib/ftl/ftl_core.h | 15 ++++++--- test/unit/lib/ftl/ftl_ppa/ftl_ppa_ut.c | 45 ++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lib/ftl/ftl_core.h b/lib/ftl/ftl_core.h index ca0d3852c..60a3fbba7 100644 --- a/lib/ftl/ftl_core.h +++ b/lib/ftl/ftl_core.h @@ -307,10 +307,17 @@ ftl_ppa_cached(struct ftl_ppa ppa) static inline uint64_t ftl_ppa_addr_pack(const struct spdk_ftl_dev *dev, struct ftl_ppa ppa) { - return (ppa.lbk << dev->ppaf.lbk_offset) | - (ppa.chk << dev->ppaf.chk_offset) | - (ppa.pu << dev->ppaf.pu_offset) | - (ppa.grp << dev->ppaf.grp_offset); + uint64_t lbk, chk, pu, grp; + + lbk = ppa.lbk; + chk = ppa.chk; + pu = ppa.pu; + grp = ppa.grp; + + return (lbk << dev->ppaf.lbk_offset) | + (chk << dev->ppaf.chk_offset) | + (pu << dev->ppaf.pu_offset) | + (grp << dev->ppaf.grp_offset); } static inline struct ftl_ppa diff --git a/test/unit/lib/ftl/ftl_ppa/ftl_ppa_ut.c b/test/unit/lib/ftl/ftl_ppa/ftl_ppa_ut.c index 2f08183bf..96262cb49 100644 --- a/test/unit/lib/ftl/ftl_ppa/ftl_ppa_ut.c +++ b/test/unit/lib/ftl/ftl_ppa/ftl_ppa_ut.c @@ -112,7 +112,7 @@ cleanup(void) } static void -test_ppa_pack(void) +test_ppa_pack32(void) { struct ftl_ppa orig = {}, ppa; @@ -148,6 +148,45 @@ test_ppa_pack(void) clean_l2p(); } +static void +test_ppa_pack64(void) +{ + struct ftl_ppa orig = {}, ppa; + + orig.lbk = 4; + orig.chk = 3; + orig.pu = 2; + orig.grp = 1; + + /* Check valid address transformation */ + ppa.ppa = ftl_ppa_addr_pack(g_dev, orig); + ppa = ftl_ppa_addr_unpack(g_dev, ppa.ppa); + CU_ASSERT_FALSE(ftl_ppa_invalid(ppa)); + CU_ASSERT_EQUAL(ppa.ppa, orig.ppa); + + orig.lbk = 0x7ea0be0f; + orig.chk = 0x6; + orig.pu = 0x4; + orig.grp = 0x2; + + ppa.ppa = ftl_ppa_addr_pack(g_dev, orig); + ppa = ftl_ppa_addr_unpack(g_dev, ppa.ppa); + CU_ASSERT_FALSE(ftl_ppa_invalid(ppa)); + CU_ASSERT_EQUAL(ppa.ppa, orig.ppa); + + /* Check maximum valid address for ppaf */ + orig.lbk = 0x7fffffff; + orig.chk = 0xf; + orig.pu = 0x7; + orig.grp = 0x3; + + ppa.ppa = ftl_ppa_addr_pack(g_dev, orig); + ppa = ftl_ppa_addr_unpack(g_dev, ppa.ppa); + CU_ASSERT_FALSE(ftl_ppa_invalid(ppa)); + CU_ASSERT_EQUAL(ppa.ppa, orig.ppa); + clean_l2p(); +} + static void test_ppa_trans(void) { @@ -248,7 +287,7 @@ main(int argc, char **argv) if ( CU_add_test(suite32, "test_ppa_pack", - test_ppa_pack) == NULL + test_ppa_pack32) == NULL || CU_add_test(suite32, "test_ppa32_invalid", test_ppa_invalid) == NULL || CU_add_test(suite32, "test_ppa32_trans", @@ -261,6 +300,8 @@ main(int argc, char **argv) test_ppa_trans) == NULL || CU_add_test(suite64, "test_ppa64_cached", test_ppa_cached) == NULL + || CU_add_test(suite64, "test_ppa64_pack", + test_ppa_pack64) == NULL ) { CU_cleanup_registry(); return CU_get_error();