diff mbox

[v2,1/2] block: add BH_Meta flag

Message ID 1337257290-28547-1-git-send-email-saugata.das@stericsson.com
State New
Headers show

Commit Message

Saugata Das May 17, 2012, 12:21 p.m. UTC
From: Saugata Das <saugata.das@linaro.org>

Today, storage devices like eMMC has special features like data tagging
(introduced in MMC-4.5 version) in order to improve performance of some
specific writes. On MMC stack, data tagging is used for all writes which
has REQ_META flag set. This patch adds the capability to add REQ_META flag
during meta data write.

Signed-off-by: Saugata Das <saugata.das@linaro.org>

changes in v2:
	Replaced the conditionals around submit_bh as suggested in the review
	comments from Boaz
---
 fs/buffer.c                 |    6 ++++--
 include/linux/buffer_head.h |    2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Saugata Das May 22, 2012, 4:18 a.m. UTC | #1
Hi Ted, Christoph

Will you please provide your feedback for these patch set,
[PATCH v2 1/2] block: add BH_Meta flag
[PATCH v2 2/2] ext4: annotate all meta data requests

If there is no comment then will you please merge them.


Regards
Saugata


On 17 May 2012 17:51, Saugata Das <saugata.das@stericsson.com> wrote:
> From: Saugata Das <saugata.das@linaro.org>
>
> Today, storage devices like eMMC has special features like data tagging
> (introduced in MMC-4.5 version) in order to improve performance of some
> specific writes. On MMC stack, data tagging is used for all writes which
> has REQ_META flag set. This patch adds the capability to add REQ_META flag
> during meta data write.
>
> Signed-off-by: Saugata Das <saugata.das@linaro.org>
>
> changes in v2:
>        Replaced the conditionals around submit_bh as suggested in the review
>        comments from Boaz
> ---
>  fs/buffer.c                 |    6 ++++--
>  include/linux/buffer_head.h |    2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 36d6665..942c75b 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -1685,7 +1685,8 @@ static int __block_write_full_page(struct inode *inode, struct page *page,
>        do {
>                struct buffer_head *next = bh->b_this_page;
>                if (buffer_async_write(bh)) {
> -                       submit_bh(write_op, bh);
> +                       submit_bh(write_op |
> +                               (buffer_meta(bh) << __REQ_META), bh);
>                        nr_underway++;
>                }
>                bh = next;
> @@ -1739,7 +1740,8 @@ recover:
>                struct buffer_head *next = bh->b_this_page;
>                if (buffer_async_write(bh)) {
>                        clear_buffer_dirty(bh);
> -                       submit_bh(write_op, bh);
> +                       submit_bh(write_op |
> +                               (buffer_meta(bh) << __REQ_META), bh);
>                        nr_underway++;
>                }
>                bh = next;
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index 458f497..13bba17 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -34,6 +34,7 @@ enum bh_state_bits {
>        BH_Write_EIO,   /* I/O error on write */
>        BH_Unwritten,   /* Buffer is allocated on disk but not written */
>        BH_Quiet,       /* Buffer Error Prinks to be quiet */
> +       BH_Meta,        /* Is meta */
>
>        BH_PrivateStart,/* not a state bit, but the first bit available
>                         * for private allocation by other entities
> @@ -124,6 +125,7 @@ BUFFER_FNS(Delay, delay)
>  BUFFER_FNS(Boundary, boundary)
>  BUFFER_FNS(Write_EIO, write_io_error)
>  BUFFER_FNS(Unwritten, unwritten)
> +BUFFER_FNS(Meta, meta)
>
>  #define bh_offset(bh)          ((unsigned long)(bh)->b_data & ~PAGE_MASK)
>  #define touch_buffer(bh)       mark_page_accessed(bh->b_page)
> --
> 1.7.4.3
>
Jeff Moyer May 22, 2012, 4:37 p.m. UTC | #2
Saugata Das <saugata.das@stericsson.com> writes:

> From: Saugata Das <saugata.das@linaro.org>
>
> Today, storage devices like eMMC has special features like data tagging
> (introduced in MMC-4.5 version) in order to improve performance of some
> specific writes. On MMC stack, data tagging is used for all writes which
> has REQ_META flag set. This patch adds the capability to add REQ_META flag
> during meta data write.

Presumably you're doing this to get better performance for some
workload, yes?  Could you please let us know what workloads you tested
and how this patch set helps?  In other words, what benchmarking did you
perform and what were the results?  Do you expect this to make some
other workloads perform worse?

Cheers,
Jeff
Saugata Das May 28, 2012, 2:30 p.m. UTC | #3
On 22 May 2012 22:07, Jeff Moyer <jmoyer@redhat.com> wrote:
> Saugata Das <saugata.das@stericsson.com> writes:
>
>> From: Saugata Das <saugata.das@linaro.org>
>>
>> Today, storage devices like eMMC has special features like data tagging
>> (introduced in MMC-4.5 version) in order to improve performance of some
>> specific writes. On MMC stack, data tagging is used for all writes which
>> has REQ_META flag set. This patch adds the capability to add REQ_META flag
>> during meta data write.
>
> Presumably you're doing this to get better performance for some
> workload, yes?  Could you please let us know what workloads you tested
> and how this patch set helps?  In other words, what benchmarking did you
> perform and what were the results?  Do you expect this to make some
> other workloads perform worse?
>

Sorry for the late reply.

The use cases, we are trying to improve are the file writes to eMMC by
redirecting the meta-data to a dedicated location within eMMC which
provides high write performance. With the proposed patch, I see that
for a 128KB file write, ~11% data writes are meta-data write. If we
consider, eMMC provides atleast double the performance for such writes
then this fetches ~5% improvement in overall file write performance.
Lower the file size, higher will be the gain. Note that eMMC
specification does not say how the memory devices will implement this
data tag feature, so the improvement seen will vary between devices
from different vendors.

There is no negative impact on any other use cases.


> Cheers,
> Jeff
diff mbox

Patch

diff --git a/fs/buffer.c b/fs/buffer.c
index 36d6665..942c75b 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1685,7 +1685,8 @@  static int __block_write_full_page(struct inode *inode, struct page *page,
 	do {
 		struct buffer_head *next = bh->b_this_page;
 		if (buffer_async_write(bh)) {
-			submit_bh(write_op, bh);
+			submit_bh(write_op |
+				(buffer_meta(bh) << __REQ_META), bh);
 			nr_underway++;
 		}
 		bh = next;
@@ -1739,7 +1740,8 @@  recover:
 		struct buffer_head *next = bh->b_this_page;
 		if (buffer_async_write(bh)) {
 			clear_buffer_dirty(bh);
-			submit_bh(write_op, bh);
+			submit_bh(write_op |
+				(buffer_meta(bh) << __REQ_META), bh);
 			nr_underway++;
 		}
 		bh = next;
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 458f497..13bba17 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -34,6 +34,7 @@  enum bh_state_bits {
 	BH_Write_EIO,	/* I/O error on write */
 	BH_Unwritten,	/* Buffer is allocated on disk but not written */
 	BH_Quiet,	/* Buffer Error Prinks to be quiet */
+	BH_Meta,	/* Is meta */
 
 	BH_PrivateStart,/* not a state bit, but the first bit available
 			 * for private allocation by other entities
@@ -124,6 +125,7 @@  BUFFER_FNS(Delay, delay)
 BUFFER_FNS(Boundary, boundary)
 BUFFER_FNS(Write_EIO, write_io_error)
 BUFFER_FNS(Unwritten, unwritten)
+BUFFER_FNS(Meta, meta)
 
 #define bh_offset(bh)		((unsigned long)(bh)->b_data & ~PAGE_MASK)
 #define touch_buffer(bh)	mark_page_accessed(bh->b_page)