Re: [PATCH] direct-io: allow direct writes to empty inodes

[Jan Kara <jack@suse.cz>]

On Mon 08-10-18 20:58:23, Ernesto A. Fern�ndez wrote:
> On a DIO_SKIP_HOLES filesystem, the ->get_block() method is currently
> not allowed to create blocks for an empty inode.  This confusion comes
> from trying to bit shift a negative number, so check the size of the
> inode first.
> 
> The problem is most visible for hfsplus, because the fallback to
> buffered I/O doesn't happen and the write fails with EIO.  This is in
> part the fault of the module, because it gives a wrong return value on
> ->get_block(); that will be fixed in a separate patch.
> 
> Signed-off-by: Ernesto A. Fern�ndez <ernesto.mnd.fernandez@gmail.com>

Good catch. The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

Also Jens often picks up patches for direct IO code so added him to CC.
Jens?

								Honza

> ---
>  fs/direct-io.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 093fb54cd316..9a7b91a3b7a7 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -679,6 +679,7 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio,
>  	unsigned long fs_count;	/* Number of filesystem-sized blocks */
>  	int create;
>  	unsigned int i_blkbits = sdio->blkbits + sdio->blkfactor;
> +	loff_t i_size;
>  
>  	/*
>  	 * If there was a memory error and we've overwritten all the
> @@ -708,8 +709,8 @@ static int get_more_blocks(struct dio *dio, struct dio_submit *sdio,
>  		 */
>  		create = dio->op == REQ_OP_WRITE;
>  		if (dio->flags & DIO_SKIP_HOLES) {
> -			if (fs_startblk <= ((i_size_read(dio->inode) - 1) >>
> -							i_blkbits))
> +			i_size = i_size_read(dio->inode);
> +			if (i_size && fs_startblk <= (i_size - 1) >> i_blkbits)
>  				create = 0;
>  		}
>  
> -- 
> 2.11.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR