linux-ext4 - Re: [PATCH v2] ext4: Create helper function for EXT4_IO_END_UNWRITTEN and i_aiodio

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <4EA7BB1F.5060008@tao.ma>
Date:	Wed, 26 Oct 2011 15:47:43 +0800
From:	Tao Ma <tm@....ma>
To:	linux-ext4@...r.kernel.org
CC:	Ted Ts'o <tytso@....edu>
Subject: Re: [PATCH v2] ext4: Create helper function for EXT4_IO_END_UNWRITTEN
 and i_aiodio_unwritten.

On 09/30/2011 05:41 PM, Tao Ma wrote:
> From: Tao Ma <boyu.mt@...bao.com>
> 
> EXT4_IO_END_UNWRITTEN flag set and the increase of i_aiodio_unwritten
> should be done simultaneously since ext4_end_io_nolock always clear the
> flag and decrease the counter in the same time.
> 
> We have found some bugs that the flag is set while leaving
> i_aiodio_unwritten unchanged(commit 32c80b32c053d). So this patch just tries
> to create a helper function to wrap them to avoid any future bug.
> The idea is inspired by Eric.
> 
> Cc: Eric Sandeen <sandeen@...hat.com>
> Signed-off-by: Tao Ma <boyu.mt@...bao.com>
ping?
> ---
> This patch is based on another bug fix which hasn't been merged yet.
> http://marc.info/?l=linux-ext4&m=131598536608613&w=2
> 
>  fs/ext4/ext4.h    |    9 +++++++++
>  fs/ext4/extents.c |   18 ++++++------------
>  fs/ext4/inode.c   |    5 +----
>  fs/ext4/page-io.c |    6 ++----
>  4 files changed, 18 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 4cb030c..a9325f5 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1241,6 +1241,15 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino)
>  		 ino <= le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count));
>  }
>  
> +static inline void ext4_set_io_unwritten_flag(struct inode *inode,
> +					      struct ext4_io_end *io_end)
> +{
> +	if (!(io_end->flag & EXT4_IO_END_UNWRITTEN)) {
> +		io_end->flag |= EXT4_IO_END_UNWRITTEN;
> +		atomic_inc(&EXT4_I(inode)->i_aiodio_unwritten);
> +	}
> +}
> +
>  /*
>   * Inode dynamic state flags
>   */
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 8f23589..31750af 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3190,12 +3190,9 @@ ext4_ext_handle_uninitialized_extents(handle_t *handle, struct inode *inode,
>  		 * that this IO needs to conversion to written when IO is
>  		 * completed
>  		 */
> -		if (io) {
> -			if (!(io->flag & EXT4_IO_END_UNWRITTEN)) {
> -				io->flag = EXT4_IO_END_UNWRITTEN;
> -				atomic_inc(&EXT4_I(inode)->i_aiodio_unwritten);
> -			}
> -		} else
> +		if (io)
> +			ext4_set_io_unwritten_flag(inode, io);
> +		else
>  			ext4_set_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN);
>  		if (ext4_should_dioread_nolock(inode))
>  			map->m_flags |= EXT4_MAP_UNINIT;
> @@ -3574,12 +3571,9 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>  		 * that we need to perform conversion when IO is done.
>  		 */
>  		if ((flags & EXT4_GET_BLOCKS_PRE_IO)) {
> -			if (io) {
> -				if (!(io->flag & EXT4_IO_END_UNWRITTEN)) {
> -					io->flag = EXT4_IO_END_UNWRITTEN;
> -					atomic_inc(&EXT4_I(inode)->i_aiodio_unwritten);
> -				}
> -			} else
> +			if (io)
> +				ext4_set_io_unwritten_flag(inode, io);
> +			else
>  				ext4_set_inode_state(inode,
>  						     EXT4_STATE_DIO_UNWRITTEN);
>  		}
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 4b37501..5d6fc9b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2685,10 +2685,7 @@ static void ext4_end_io_buffer_write(struct buffer_head *bh, int uptodate)
>  	 * but being more careful is always safe for the future change.
>  	 */
>  	inode = io_end->inode;
> -	if (!(io_end->flag & EXT4_IO_END_UNWRITTEN)) {
> -		io_end->flag |= EXT4_IO_END_UNWRITTEN;
> -		atomic_inc(&EXT4_I(inode)->i_aiodio_unwritten);
> -	}
> +	ext4_set_io_unwritten_flag(inode, io_end);
>  
>  	/* Add the io_end to per-inode completed io list*/
>  	spin_lock_irqsave(&EXT4_I(inode)->i_completed_io_lock, flags);
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 92f38ee..f5eec5e 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -350,10 +350,8 @@ submit_and_retry:
>  	if ((io_end->num_io_pages >= MAX_IO_PAGES) &&
>  	    (io_end->pages[io_end->num_io_pages-1] != io_page))
>  		goto submit_and_retry;
> -	if (buffer_uninit(bh) && !(io_end->flag & EXT4_IO_END_UNWRITTEN)) {
> -		io_end->flag |= EXT4_IO_END_UNWRITTEN;
> -		atomic_inc(&EXT4_I(inode)->i_aiodio_unwritten);
> -	}
> +	if (buffer_uninit(bh))
> +		ext4_set_io_unwritten_flag(inode, io_end);
>  	io->io_end->size += bh->b_size;
>  	io->io_next_block++;
>  	ret = bio_add_page(io->io_bio, bh->b_page, bh->b_size, bh_offset(bh));

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