| Message ID | 1330692309-6187-1-git-send-email-saugata.das@stericsson.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
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
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
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
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)