Re: [RFC] AFFS: Trying to fix fsx/O_DIRECT

[Jan Kara <jack@suse.cz>]

  Hello,

On Fri 02-01-15 17:08:11, Fabian Frederick wrote:
> xfstests/ltp/fsx version (fsx file -d -Z -r 4096 -w 4096)
> always ends up with filesystem being remounted read-only due to a
> write operation done beyond truncate offset.
> 
> affs debug:
> affs: error (device sdd1): get_block(): strange block request 136
> affs: Remounting filesystem read-only
> 
> Fsx report:
> "
> 129(129 mod 256): TRUNCATE DOWN from 0x3ff01 to 0xb3f6
> 130(130 mod 256): WRITE    0x22000 thru 0x2dfff (0xc000 bytes) HOLE
> "
> 
> Error in affs_get_block:
> if (block >= AFFS_I(inode)->i_blkcnt) {
>                 if (block > AFFS_I(inode)->i_blkcnt || !create)
>                         goto err_big;
>         } else
>                 create = 0;
> 
> When I display values there, block is 136; i_blkcnt: 45
> 
> It seems operations are mixed up and especially truncate/get_block.
> I tried the patch below to add truncate mutex featuring in some other
> filesystems but problem remains the same.
> Is there something else I could do to avoid such problem ?
  As I wrote in another email I think this is a problem with direct IO
write beyond EOF. Specifically, I don't think this is a problem with
locking since fsx is singlethreaded...

								Honza

> ---
>  fs/affs/affs.h  |  1 +
>  fs/affs/file.c  | 10 +++++++++-
>  fs/affs/super.c |  1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/affs/affs.h b/fs/affs/affs.h
> index ff44ff3..c7942d9 100644
> --- a/fs/affs/affs.h
> +++ b/fs/affs/affs.h
> @@ -56,6 +56,7 @@ struct affs_inode_info {
>  	u32	 i_protect;			/* unused attribute bits */
>  	u32	 i_lastalloc;			/* last allocated block */
>  	int	 i_pa_cnt;			/* number of preallocated blocks */
> +	struct mutex truncate_mutex;
>  	struct inode vfs_inode;
>  };
>  
> diff --git a/fs/affs/file.c b/fs/affs/file.c
> index 8faa659..7e1ab3e 100644
> --- a/fs/affs/file.c
> +++ b/fs/affs/file.c
> @@ -321,7 +321,10 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
>  	map_bh(bh_result, sb, (sector_t)be32_to_cpu(AFFS_BLOCK(sb, ext_bh, block)));
>  
>  	if (create) {
> -		u32 blocknr = affs_alloc_block(inode, ext_bh->b_blocknr);
> +		u32 blocknr;
> +
> +		mutex_lock(&AFFS_I(inode)->truncate_mutex);
> +		blocknr = affs_alloc_block(inode, ext_bh->b_blocknr);
>  		if (!blocknr)
>  			goto err_alloc;
>  		set_buffer_new(bh_result);
> @@ -349,6 +352,7 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
>  
>  	affs_brelse(ext_bh);
>  	//unlock cache
> +	mutex_unlock(&AFFS_I(inode)->truncate_mutex);
>  	affs_unlock_ext(inode);
>  	return 0;
>  
> @@ -365,6 +369,7 @@ err_alloc:
>  	clear_buffer_mapped(bh_result);
>  	bh_result->b_bdev = NULL;
>  	// unlock cache
> +	mutex_unlock(&AFFS_I(inode)->truncate_mutex);
>  	affs_unlock_ext(inode);
>  	return -ENOSPC;
>  }
> @@ -860,6 +865,7 @@ affs_truncate(struct inode *inode)
>  
>  	// lock cache
>  	ext_bh = affs_get_extblock(inode, ext);
> +	mutex_lock(&AFFS_I(inode)->truncate_mutex);
>  	if (IS_ERR(ext_bh)) {
>  		affs_warning(sb, "truncate",
>  			     "unexpected read error for ext block %u (%ld)",
> @@ -912,6 +918,7 @@ affs_truncate(struct inode *inode)
>  				affs_warning(sb, "truncate",
>  					     "unexpected read error for last block %u (%ld)",
>  					     (unsigned int)ext, PTR_ERR(bh));
> +				mutex_unlock(&AFFS_I(inode)->truncate_mutex);
>  				return;
>  			}
>  			tmp = be32_to_cpu(AFFS_DATA_HEAD(bh)->next);
> @@ -938,6 +945,7 @@ affs_truncate(struct inode *inode)
>  		affs_brelse(ext_bh);
>  	}
>  	affs_free_prealloc(inode);
> +	mutex_unlock(&AFFS_I(inode)->truncate_mutex);
>  }
>  
>  int affs_file_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
> diff --git a/fs/affs/super.c b/fs/affs/super.c
> index f754ab6..66c74df 100644
> --- a/fs/affs/super.c
> +++ b/fs/affs/super.c
> @@ -123,6 +123,7 @@ static void init_once(void *foo)
>  
>  	sema_init(&ei->i_link_lock, 1);
>  	sema_init(&ei->i_ext_lock, 1);
> +	mutex_init(&ei->truncate_mutex);
>  	inode_init_once(&ei->vfs_inode);
>  }
>  
> -- 
> 2.1.0
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR