Patchwork [v2] REQ_META set during meta data write

login
register
mail settings
Submitter Saugata Das
Date March 2, 2012, 12:45 p.m.
Message ID <1330692309-6187-1-git-send-email-saugata.das@stericsson.com>
Download mbox | patch
Permalink /patch/144241/
State New
Headers show

Comments

Saugata Das - March 2, 2012, 12:45 p.m.
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. On EXT4, however, currently REQ_META is set only for read.
This patch adds the capability to add REQ_META flag during meta data write.
This patch adds new function set_buffer_meta, which is used to set a bit
BH_Meta in the b_state field when meta data and super block is made dirty.

Signed-off-by: Saugata Das <saugata.das@linaro.org>
Acked-by: Arnd Bergmann <arnd.bergmann@linaro.org>

Changelog v2:
	- Removed check for WRITE when setting REQ_META in submit_bh
Changelog v1:
	- Introduction of REQ_META during write
---
 fs/buffer.c                 |    3 +++
 fs/ext4/ext4_jbd2.c         |    4 ++++
 fs/ext4/inode.c             |    4 +++-
 include/linux/buffer_head.h |    2 ++
 4 files changed, 12 insertions(+), 1 deletions(-)
Christoph Hellwig - March 3, 2012, 9:34 a.m.
On Fri, Mar 02, 2012 at 06:15:09PM +0530, Saugata Das wrote:
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2926,6 +2926,9 @@ int submit_bh(int rw, struct buffer_head * bh)
>  	if (test_set_buffer_req(bh) && (rw & WRITE))
>  		clear_buffer_write_io_error(bh);
>  
> +	if (buffer_meta(bh))
> +		rw |= REQ_META;
> +

submit_bh has a perfectly valid rw argument where you can pass REQ_META.
Please use that instead of abusing a buffer state bit for an I/O
submission flag.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Saugata Das - March 4, 2012, 8:02 a.m.
On 3 March 2012 15:04, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, Mar 02, 2012 at 06:15:09PM +0530, Saugata Das wrote:
>> --- a/fs/buffer.c
>> +++ b/fs/buffer.c
>> @@ -2926,6 +2926,9 @@ int submit_bh(int rw, struct buffer_head * bh)
>>       if (test_set_buffer_req(bh) && (rw & WRITE))
>>               clear_buffer_write_io_error(bh);
>>
>> +     if (buffer_meta(bh))
>> +             rw |= REQ_META;
>> +
>
> submit_bh has a perfectly valid rw argument where you can pass REQ_META.
> Please use that instead of abusing a buffer state bit for an I/O
> submission flag.
>

Typically, during meta-data writes, the buffer is only marked dirty
(__ext4_handle_dirty_metadata). The submit_bh of the dirty meta-data
buffer happens later from writeback thread which in general flushes
out all the buffers. From this generic handling of dirty buffers from
writeback thread via generic writepage functions till submit_bh, it is
not possible to identify whether the buffer was corresponding to any
meta-data without having an extra information in the buffer itself.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Saugata Das - March 22, 2012, 11:08 a.m.
Hi Christoph

On 4 March 2012 13:32, Saugata Das <saugata.das@linaro.org> wrote:
> On 3 March 2012 15:04, Christoph Hellwig <hch@infradead.org> wrote:
>> On Fri, Mar 02, 2012 at 06:15:09PM +0530, Saugata Das wrote:
>>> --- a/fs/buffer.c
>>> +++ b/fs/buffer.c
>>> @@ -2926,6 +2926,9 @@ int submit_bh(int rw, struct buffer_head * bh)
>>>       if (test_set_buffer_req(bh) && (rw & WRITE))
>>>               clear_buffer_write_io_error(bh);
>>>
>>> +     if (buffer_meta(bh))
>>> +             rw |= REQ_META;
>>> +
>>
>> submit_bh has a perfectly valid rw argument where you can pass REQ_META.
>> Please use that instead of abusing a buffer state bit for an I/O
>> submission flag.
>>
>
> Typically, during meta-data writes, the buffer is only marked dirty
> (__ext4_handle_dirty_metadata). The submit_bh of the dirty meta-data
> buffer happens later from writeback thread which in general flushes
> out all the buffers. From this generic handling of dirty buffers from
> writeback thread via generic writepage functions till submit_bh, it is
> not possible to identify whether the buffer was corresponding to any
> meta-data without having an extra information in the buffer itself.

Is the above explanation OK for you ? Or, do you some alternative
suggestion for me to explore ?

Based on your feedback, I shall resubmit the patch on top of 3.4.


regards
saugata
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/buffer.c b/fs/buffer.c
index 19d8eb7..6eaf492 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2926,6 +2926,9 @@  int submit_bh(int rw, struct buffer_head * bh)
 	if (test_set_buffer_req(bh) && (rw & WRITE))
 		clear_buffer_write_io_error(bh);
 
+	if (buffer_meta(bh))
+		rw |= REQ_META;
+
 	/*
 	 * from here on down, it's all bio -- do the initial mapping,
 	 * submit_bio -> generic_make_request may further map this bio around
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 92655fd..105e1d0 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4144,8 +4144,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");
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)