diff mbox series

[02/10] fs: pick out ll_rw_one_block() helper function

Message ID 20200526071754.33819-3-yi.zhang@huawei.com
State New
Headers show
Series ext4: fix inconsistency since reading old metadata from disk | expand

Commit Message

zhangyi (F) May 26, 2020, 7:17 a.m. UTC
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(-)

Comments

Christoph Hellwig May 28, 2020, 5:07 a.m. UTC | #1
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.
zhangyi (F) May 28, 2020, 1:23 p.m. UTC | #2
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 mbox series

Patch

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);