Re: [PATCH 1/4] Add buffer head related helper functions

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 15 Nov 2007 17:40:43 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:

> Add buffer head related helper function
> bh_uptodate_or_lock and bh_submit_read
> which can be used by file system
> 

The patches look sane.

> ---
>  include/linux/buffer_head.h |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index da0d83f..82cc9ef 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -327,6 +327,35 @@ static inline void lock_buffer(struct buffer_head *bh)
>  
>  extern int __set_page_dirty_buffers(struct page *page);
>  
> +/* Return true if the buffer is up-to-date.
> + * Return false, with the buffer locked, if not.
> + */
> +static inline int bh_uptodate_or_lock(struct buffer_head *bh)
> +{
> +	if (!buffer_uptodate(bh)) {
> +		lock_buffer(bh);
> +		if (!buffer_uptodate(bh))
> +			return 0;
> +		unlock_buffer(bh);
> +	}
> +	return 1;
> +}
> +/*
> + * Submit a locked buffer for reading,
> + * return a negative error and release
> + * the buffer if failed.
> + */
> +static inline int bh_submit_read(struct buffer_head *bh)
> +{
> +	get_bh(bh);
> +	bh->b_end_io = end_buffer_read_sync;
> +	submit_bh(READ, bh);
> +	wait_on_buffer(bh);
> +	if (buffer_uptodate(bh))
> +		return 0;
> +	brelse(bh);
> +	return -EIO;
> +}
>  #else /* CONFIG_BLOCK */

But these are waaaaaay too big to be inlined.  Could I ask that they be
turned into regular EXPORT_SYMBOL()ed functions in buffer.c?

Might as well turn the comments into regular kerneldoc format, too.

The function names are a bit awkward, but I can't think of anything better.

I'm surprised that we don't already have a function which does what
bh_submit_read() does.

bh_submit_read() might become a bit simpler if it called ll_rw_block().

I'd have thought that to be more general, bh_submit_read() should return
immediately if the buffer is uptodate.  ll_rw_block() sort of helps there.