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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20071116133353.f5babdd3.akpm@linux-foundation.org>
Date:	Fri, 16 Nov 2007 13:33:53 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	linux-ext4@...r.kernel.org, cmm@...ibm.com,
	torvalds@...ux-foundation.org, a.p.zijlstra@...llo.nl,
	adilger@...sterfs.com, aneesh.kumar@...ux.vnet.ibm.com
Subject: Re: [PATCH 1/4] Add buffer head related helper functions

On Thu, 15 Nov 2007 17:40:43 +0530
"Aneesh Kumar K.V" <aneesh.kumar@...ux.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.


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