| Message ID | 20200526071754.33819-3-yi.zhang@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: fix inconsistency since reading old metadata from disk | expand |
On Tue, May 26, 2020 at 03:17:46PM +0800, zhangyi (F) wrote: > Pick out ll_rw_one_block() helper function from ll_rw_block() for > submitting one locked buffer for reading/writing. That should probably read factor out instead of pick out. > > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> > --- > fs/buffer.c | 41 ++++++++++++++++++++++--------------- > include/linux/buffer_head.h | 1 + > 2 files changed, 26 insertions(+), 16 deletions(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index a60f60396cfa..3a2226f88b2d 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -3081,6 +3081,29 @@ int submit_bh(int op, int op_flags, struct buffer_head *bh) > } > EXPORT_SYMBOL(submit_bh); > > +void ll_rw_one_block(int op, int op_flags, struct buffer_head *bh) > +{ > + BUG_ON(!buffer_locked(bh)); > + > + if (op == WRITE) { > + if (test_clear_buffer_dirty(bh)) { > + bh->b_end_io = end_buffer_write_sync; > + get_bh(bh); > + submit_bh(op, op_flags, bh); > + return; > + } > + } else { > + if (!buffer_uptodate(bh)) { > + bh->b_end_io = end_buffer_read_sync; > + get_bh(bh); > + submit_bh(op, op_flags, bh); > + return; > + } > + } > + unlock_buffer(bh); > +} > +EXPORT_SYMBOL(ll_rw_one_block); I don't think you want separate read and write sides. In fact I'm not sure you want the helper at all. At this point just open coding it rather than adding more overhead to core code might be a better idea.
Hi, Christoph On 2020/5/28 13:07, Christoph Hellwig wrote: > On Tue, May 26, 2020 at 03:17:46PM +0800, zhangyi (F) wrote: >> Pick out ll_rw_one_block() helper function from ll_rw_block() for >> submitting one locked buffer for reading/writing. > > That should probably read factor out instead of pick out. > >> >> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> >> --- >> fs/buffer.c | 41 ++++++++++++++++++++++--------------- >> include/linux/buffer_head.h | 1 + >> 2 files changed, 26 insertions(+), 16 deletions(-) >> >> diff --git a/fs/buffer.c b/fs/buffer.c >> index a60f60396cfa..3a2226f88b2d 100644 >> --- a/fs/buffer.c >> +++ b/fs/buffer.c >> @@ -3081,6 +3081,29 @@ int submit_bh(int op, int op_flags, struct buffer_head *bh) >> } >> EXPORT_SYMBOL(submit_bh); >> >> +void ll_rw_one_block(int op, int op_flags, struct buffer_head *bh) >> +{ >> + BUG_ON(!buffer_locked(bh)); >> + >> + if (op == WRITE) { >> + if (test_clear_buffer_dirty(bh)) { >> + bh->b_end_io = end_buffer_write_sync; >> + get_bh(bh); >> + submit_bh(op, op_flags, bh); >> + return; >> + } >> + } else { >> + if (!buffer_uptodate(bh)) { >> + bh->b_end_io = end_buffer_read_sync; >> + get_bh(bh); >> + submit_bh(op, op_flags, bh); >> + return; >> + } >> + } >> + unlock_buffer(bh); >> +} >> +EXPORT_SYMBOL(ll_rw_one_block); > > I don't think you want separate read and write sides. In fact I'm not > sure you want the helper at all. At this point just open coding it > rather than adding more overhead to core code might be a better idea. > Yeah, what I want is only the read side, it's fine by me to open coding it. Thanks, Yi.
diff --git a/fs/buffer.c b/fs/buffer.c index a60f60396cfa..3a2226f88b2d 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -3081,6 +3081,29 @@ int submit_bh(int op, int op_flags, struct buffer_head *bh) } EXPORT_SYMBOL(submit_bh); +void ll_rw_one_block(int op, int op_flags, struct buffer_head *bh) +{ + BUG_ON(!buffer_locked(bh)); + + if (op == WRITE) { + if (test_clear_buffer_dirty(bh)) { + bh->b_end_io = end_buffer_write_sync; + get_bh(bh); + submit_bh(op, op_flags, bh); + return; + } + } else { + if (!buffer_uptodate(bh)) { + bh->b_end_io = end_buffer_read_sync; + get_bh(bh); + submit_bh(op, op_flags, bh); + return; + } + } + unlock_buffer(bh); +} +EXPORT_SYMBOL(ll_rw_one_block); + /** * ll_rw_block: low-level access to block devices (DEPRECATED) * @op: whether to %READ or %WRITE @@ -3116,22 +3139,8 @@ void ll_rw_block(int op, int op_flags, int nr, struct buffer_head *bhs[]) if (!trylock_buffer(bh)) continue; - if (op == WRITE) { - if (test_clear_buffer_dirty(bh)) { - bh->b_end_io = end_buffer_write_sync; - get_bh(bh); - submit_bh(op, op_flags, bh); - continue; - } - } else { - if (!buffer_uptodate(bh)) { - bh->b_end_io = end_buffer_read_sync; - get_bh(bh); - submit_bh(op, op_flags, bh); - continue; - } - } - unlock_buffer(bh); + + ll_rw_one_block(op, op_flags, bh); } } EXPORT_SYMBOL(ll_rw_block); diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h index 15b765a181b8..11aa412c0bcd 100644 --- a/include/linux/buffer_head.h +++ b/include/linux/buffer_head.h @@ -198,6 +198,7 @@ struct buffer_head *alloc_buffer_head(gfp_t gfp_flags); void free_buffer_head(struct buffer_head * bh); void unlock_buffer(struct buffer_head *bh); void __lock_buffer(struct buffer_head *bh); +void ll_rw_one_block(int op, int op_flags, struct buffer_head *bh); void ll_rw_block(int, int, int, struct buffer_head * bh[]); int sync_dirty_buffer(struct buffer_head *bh); int __sync_dirty_buffer(struct buffer_head *bh, int op_flags);
Pick out ll_rw_one_block() helper function from ll_rw_block() for submitting one locked buffer for reading/writing. Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> --- fs/buffer.c | 41 ++++++++++++++++++++++--------------- include/linux/buffer_head.h | 1 + 2 files changed, 26 insertions(+), 16 deletions(-)