Re: Build failures due to commit 416161db (btrfs: offline dedupe)

[Guenter Roeck]

On Tue, Sep 17, 2013 at 03:43:54PM -0700, Mark Fasheh wrote:
> On Fri, Sep 13, 2013 at 03:33:34PM -0400, Chris Mason wrote:
> > Mark, could you please send a patch for the whole-struct option until
> > the unaligned put is upstreamed?
> > 
> > -chris
> 
> Here you go. It's been lightly tested and needs review.
> 
At the very least it does fix the build error on the affected platforms.

Guenter

> Thanks,
> 	--Mark
> 
> --
> Mark Fasheh
> 
> From: Mark Fasheh <mfasheh@xxxxxxx>
> 
> [PATCH] btrfs: change extent-same to copy entire argument struct
> 
> btrfs_ioctl_file_extent_same() uses __put_user_unaligned() to copy some data
> back to it's argument struct. Unfortunately, not all architectures provide
> __put_user_unaligned(), so compiles break on them if btrfs is selected.
> 
> Instead, just copy the whole struct in / out at the start and end of
> operations, respectively.
> 
> Signed-off-by: Mark Fasheh <mfasheh@xxxxxxx>
> ---
>  fs/btrfs/ioctl.c | 76 +++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 45 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 1a5b946..25d6920 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2696,9 +2696,9 @@ out_unlock:
>  static long btrfs_ioctl_file_extent_same(struct file *file,
>  					 void __user *argp)
>  {
> -	struct btrfs_ioctl_same_args *args = argp;
> -	struct btrfs_ioctl_same_args same;
> -	struct btrfs_ioctl_same_extent_info info;
> +	struct btrfs_ioctl_same_args tmp;
> +	struct btrfs_ioctl_same_args *same;
> +	struct btrfs_ioctl_same_extent_info *info;
>  	struct inode *src = file->f_dentry->d_inode;
>  	struct file *dst_file = NULL;
>  	struct inode *dst;
> @@ -2706,6 +2706,7 @@ static long btrfs_ioctl_file_extent_same(struct file *file,
>  	u64 len;
>  	int i;
>  	int ret;
> +	unsigned long size;
>  	u64 bs = BTRFS_I(src)->root->fs_info->sb->s_blocksize;
>  	bool is_admin = capable(CAP_SYS_ADMIN);
>  
> @@ -2716,15 +2717,30 @@ static long btrfs_ioctl_file_extent_same(struct file *file,
>  	if (ret)
>  		return ret;
>  
> -	if (copy_from_user(&same,
> +	if (copy_from_user(&tmp,
>  			   (struct btrfs_ioctl_same_args __user *)argp,
> -			   sizeof(same))) {
> +			   sizeof(tmp))) {
>  		ret = -EFAULT;
>  		goto out;
>  	}
>  
> -	off = same.logical_offset;
> -	len = same.length;
> +	size = sizeof(tmp) +
> +		tmp.dest_count * sizeof(struct btrfs_ioctl_same_extent_info);
> +
> +	same = kmalloc(size, GFP_NOFS);
> +	if (!same) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	if (copy_from_user(same,
> +			   (struct btrfs_ioctl_same_args __user *)argp, size)) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	off = same->logical_offset;
> +	len = same->length;
>  
>  	/*
>  	 * Limit the total length we will dedupe for each operation.
> @@ -2752,27 +2768,28 @@ static long btrfs_ioctl_file_extent_same(struct file *file,
>  	if (!S_ISREG(src->i_mode))
>  		goto out;
>  
> -	ret = 0;
> -	for (i = 0; i < same.dest_count; i++) {
> -		if (copy_from_user(&info, &args->info[i], sizeof(info))) {
> -			ret = -EFAULT;
> -			goto out;
> -		}
> +	/* pre-format output fields to sane values */
> +	for (i = 0; i < same->dest_count; i++) {
> +		same->info[i].bytes_deduped = 0ULL;
> +		same->info[i].status = 0;
> +	}
>  
> -		info.bytes_deduped = 0;
> +	ret = 0;
> +	for (i = 0; i < same->dest_count; i++) {
> +		info = &same->info[i];
>  
> -		dst_file = fget(info.fd);
> +		dst_file = fget(info->fd);
>  		if (!dst_file) {
> -			info.status = -EBADF;
> +			info->status = -EBADF;
>  			goto next;
>  		}
>  
>  		if (!(is_admin || (dst_file->f_mode & FMODE_WRITE))) {
> -			info.status = -EINVAL;
> +			info->status = -EINVAL;
>  			goto next;
>  		}
>  
> -		info.status = -EXDEV;
> +		info->status = -EXDEV;
>  		if (file->f_path.mnt != dst_file->f_path.mnt)
>  			goto next;
>  
> @@ -2781,32 +2798,29 @@ static long btrfs_ioctl_file_extent_same(struct file *file,
>  			goto next;
>  
>  		if (S_ISDIR(dst->i_mode)) {
> -			info.status = -EISDIR;
> +			info->status = -EISDIR;
>  			goto next;
>  		}
>  
>  		if (!S_ISREG(dst->i_mode)) {
> -			info.status = -EACCES;
> +			info->status = -EACCES;
>  			goto next;
>  		}
>  
> -		info.status = btrfs_extent_same(src, off, len, dst,
> -						info.logical_offset);
> -		if (info.status == 0)
> -			info.bytes_deduped += len;
> +		info->status = btrfs_extent_same(src, off, len, dst,
> +						info->logical_offset);
> +		if (info->status == 0)
> +			info->bytes_deduped += len;
>  
>  next:
>  		if (dst_file)
>  			fput(dst_file);
> -
> -		if (__put_user_unaligned(info.status, &args->info[i].status) ||
> -		    __put_user_unaligned(info.bytes_deduped,
> -					 &args->info[i].bytes_deduped)) {
> -			ret = -EFAULT;
> -			goto out;
> -		}                                                               
>  	}
>  
> +	ret = copy_to_user(argp, same, size);
> +	if (ret)
> +		ret = -EFAULT;
> +
>  out:
>  	mnt_drop_write_file(file);
>  	return ret;
> -- 
> 1.8.1.4
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/