2012-05-17 12:21:29

by Saugata Das

[permalink] [raw]
Subject: [PATCH v2 1/2] block: add BH_Meta flag

From: Saugata Das <[email protected]>

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 <[email protected]>

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



2012-05-17 12:22:36

by Saugata Das

[permalink] [raw]
Subject: [PATCH v2 2/2] ext4: annotate all meta data requests

From: Saugata Das <[email protected]>

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. On EXT4, however, currently REQ_META is set only for read.
This patch adds the capability mark a meta-data buffer with set_buffer_meta
during meta data write. During submit_bh, this information is used to set
REQ_META flag.

Signed-off-by: Saugata Das <[email protected]>
---
fs/ext4/ext4_jbd2.c | 4 ++++
fs/ext4/inode.c | 4 +++-
2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index aca1790..097c062 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -107,6 +107,8 @@ int __ext4_handle_dirty_metadata(const char *where, unsigned int line,
{
int err = 0;

+ set_buffer_meta(bh);
+
if (ext4_handle_valid(handle)) {
err = jbd2_journal_dirty_metadata(handle, bh);
if (err) {
@@ -143,6 +145,8 @@ int __ext4_handle_dirty_super(const char *where, unsigned int line,
struct buffer_head *bh = EXT4_SB(sb)->s_sbh;
int err = 0;

+ set_buffer_meta(bh);
+
if (ext4_handle_valid(handle)) {
err = jbd2_journal_dirty_metadata(handle, bh);
if (err)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c77b0bd..754fe77 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4035,8 +4035,10 @@ int ext4_write_inode(struct inode *inode, struct writeback_control *wbc)
err = __ext4_get_inode_loc(inode, &iloc, 0);
if (err)
return err;
- if (wbc->sync_mode == WB_SYNC_ALL)
+ if (wbc->sync_mode == WB_SYNC_ALL) {
+ set_buffer_meta(iloc.bh);
sync_dirty_buffer(iloc.bh);
+ }
if (buffer_req(iloc.bh) && !buffer_uptodate(iloc.bh)) {
EXT4_ERROR_INODE_BLOCK(inode, iloc.bh->b_blocknr,
"IO error syncing inode");
--
1.7.4.3


2012-05-22 04:18:24

by Saugata Das

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] block: add BH_Meta flag

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 <[email protected]> wrote:
> From: Saugata Das <[email protected]>
>
> 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 <[email protected]>
>
> 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
>

2012-05-22 16:37:54

by Jeff Moyer

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] block: add BH_Meta flag

Saugata Das <[email protected]> writes:

> From: Saugata Das <[email protected]>
>
> 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

2012-05-28 14:30:38

by Saugata Das

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] block: add BH_Meta flag

On 22 May 2012 22:07, Jeff Moyer <[email protected]> wrote:
> Saugata Das <[email protected]> writes:
>
>> From: Saugata Das <[email protected]>
>>
>> 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