[RFC PATCH v2] mmc: remove bondage between REQ_META and reliable write

[RFC PATCH v2] mmc: remove bondage between REQ_META and reliable write

[Luca Porzio (lporzio)]

mmc: remove bondage between REQ_META and reliable write

Anytime a write operation is performed with Reliable Write flag enabled, 
the eMMC device is enforced to bypass the cache and do a write to the 
underling NVM device by Jedec specification; this causes a performance 
penalty since write operations cannot be optimized by the device cache.

In our tests, we replayed a typical mobile daily trace pattern and 
found ~9% overall time reduction in trace replay by using this patch.
Also the write ops within 4KB~64KB chunk size range get a 40~60% 
performance improvement by using the patch (as this range of write 
chunks are the one affected by REQ_META).

This patch has been discussed in the Mobile & Embedded Linux Storage 
Forum and it is the results of feedbacks from many people.
We also checked with fsdevl and f2fs mailing list developers that this change 
in the usage of REQ_META is not affecting FS behavior and we got 
positive feedbacks on applying this patch.
Reporting here the feedbacks:
- http://comments.gmane.org/gmane.linux.file-systems/97219
- http://thread.gmane.org/gmane.linux.file-systems.f2fs/3178/focus=3183

Signed-off-by: Bruce Ford <bford@micron.com>
Signed-off-by: Luca Porzio <lporzio@micron.com>
---
 drivers/mmc/card/block.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 4409d79..cba6d5e 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -62,8 +62,7 @@ MODULE_ALIAS("mmc:block");
 #define MMC_SANITIZE_REQ_TIMEOUT 240000
 #define MMC_EXTRACT_INDEX_FROM_ARG(x) ((x & 0x00FF0000) >> 16)
 
-#define mmc_req_rel_wr(req)	(((req->cmd_flags & REQ_FUA) || \
-				  (req->cmd_flags & REQ_META)) && \
+#define mmc_req_rel_wr(req)	((req->cmd_flags & REQ_FUA) && \
 				  (rq_data_dir(req) == WRITE))
 #define PACKED_CMD_VER	0x01
 #define PACKED_CMD_WR	0x02
@@ -1353,13 +1352,9 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 
 	/*
 	 * Reliable writes are used to implement Forced Unit Access and
-	 * REQ_META accesses, and are supported only on MMCs.
-	 *
-	 * XXX: this really needs a good explanation of why REQ_META
-	 * is treated special.
+	 * are supported only on MMCs.
 	 */
-	bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
-			  (req->cmd_flags & REQ_META)) &&
+	bool do_rel_wr = (req->cmd_flags & REQ_FUA) &&
 		(rq_data_dir(req) == WRITE) &&
 		(md->flags & MMC_BLK_REL_WR);
 
-- 
2.1.1

Re: [RFC PATCH v2] mmc: remove bondage between REQ_META and reliable write

[Ulf Hansson]

On 6 November 2015 at 16:12, Luca Porzio (lporzio) <lporzio@micron.com> wrote:
> mmc: remove bondage between REQ_META and reliable write
>
> Anytime a write operation is performed with Reliable Write flag enabled,
> the eMMC device is enforced to bypass the cache and do a write to the
> underling NVM device by Jedec specification; this causes a performance
> penalty since write operations cannot be optimized by the device cache.
>
> In our tests, we replayed a typical mobile daily trace pattern and
> found ~9% overall time reduction in trace replay by using this patch.
> Also the write ops within 4KB~64KB chunk size range get a 40~60%
> performance improvement by using the patch (as this range of write
> chunks are the one affected by REQ_META).
>
> This patch has been discussed in the Mobile & Embedded Linux Storage
> Forum and it is the results of feedbacks from many people.
> We also checked with fsdevl and f2fs mailing list developers that this change
> in the usage of REQ_META is not affecting FS behavior and we got
> positive feedbacks on applying this patch.
> Reporting here the feedbacks:
> - http://comments.gmane.org/gmane.linux.file-systems/97219
> - http://thread.gmane.org/gmane.linux.file-systems.f2fs/3178/focus=3183
>
> Signed-off-by: Bruce Ford <bford@micron.com>
> Signed-off-by: Luca Porzio <lporzio@micron.com>

I browsed the history to understand when the behaviour of REQ_META was
introduced. It's really strange as it seems to slipped through when
"packed command" support was added [1].
I couldn't find any related information to why, so probably it was
just a mistake. For that reason, I decided to add a fixes tag to the
commit message for $subject patch.

Moreover, I also updated minor parts in the change log to silence some
checkpatch warnings.

Applied for fixes, thanks!

Kind regards
Uffe

[1]
commit ce39f9d17c14e56ea6772aa84393e6e0cc8499c4
Author: Seungwon Jeon <tgih.jun@samsung.com>
Date:   Wed Feb 6 17:02:46 2013 +0900
mmc: support packed write command for eMMC4.5 devices

> ---
>  drivers/mmc/card/block.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 4409d79..cba6d5e 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -62,8 +62,7 @@ MODULE_ALIAS("mmc:block");
>  #define MMC_SANITIZE_REQ_TIMEOUT 240000
>  #define MMC_EXTRACT_INDEX_FROM_ARG(x) ((x & 0x00FF0000) >> 16)
>
> -#define mmc_req_rel_wr(req)    (((req->cmd_flags & REQ_FUA) || \
> -                                 (req->cmd_flags & REQ_META)) && \
> +#define mmc_req_rel_wr(req)    ((req->cmd_flags & REQ_FUA) && \
>                                   (rq_data_dir(req) == WRITE))
>  #define PACKED_CMD_VER 0x01
>  #define PACKED_CMD_WR  0x02
> @@ -1353,13 +1352,9 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>
>         /*
>          * Reliable writes are used to implement Forced Unit Access and
> -        * REQ_META accesses, and are supported only on MMCs.
> -        *
> -        * XXX: this really needs a good explanation of why REQ_META
> -        * is treated special.
> +        * are supported only on MMCs.
>          */
> -       bool do_rel_wr = ((req->cmd_flags & REQ_FUA) ||
> -                         (req->cmd_flags & REQ_META)) &&
> +       bool do_rel_wr = (req->cmd_flags & REQ_FUA) &&
>                 (rq_data_dir(req) == WRITE) &&
>                 (md->flags & MMC_BLK_REL_WR);
>
> --
> 2.1.1
>