We were incorrectly taking the async path even for the sync ioctls by passing in &transid unconditionally. There''s ample room for further cleanup here, but this keeps the fix simple. Signed-off-by: Sage Weil <sage@newdream.net> --- fs/btrfs/ioctl.c | 20 +++++++++++--------- 1 files changed, 11 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 41614c3..4c2d7c4 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -970,6 +970,15 @@ static noinline int btrfs_ioctl_snap_create(struct file *file, name = async_vol_args->name; fd = async_vol_args->fd; async_vol_args->name[BTRFS_SNAPSHOT_NAME_MAX] = ''\0''; + + ret = btrfs_ioctl_snap_create_transid(file, name, fd, + subvol, &transid); + + if (ret == 0 && + copy_to_user(arg + + offsetof(struct btrfs_ioctl_async_vol_args, + transid), &transid, sizeof(transid))) + ret = -EFAULT; } else { vol_args = memdup_user(arg, sizeof(*vol_args)); if (IS_ERR(vol_args)) @@ -977,16 +986,9 @@ static noinline int btrfs_ioctl_snap_create(struct file *file, name = vol_args->name; fd = vol_args->fd; vol_args->name[BTRFS_PATH_NAME_MAX] = ''\0''; - } - - ret = btrfs_ioctl_snap_create_transid(file, name, fd, - subvol, &transid); - if (!ret && async) { - if (copy_to_user(arg + - offsetof(struct btrfs_ioctl_async_vol_args, - transid), &transid, sizeof(transid))) - return -EFAULT; + ret = btrfs_ioctl_snap_create_transid(file, name, fd, + subvol, NULL); } kfree(vol_args); -- 1.6.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sage Weil wrote:> We were incorrectly taking the async path even for the sync ioctls by > passing in &transid unconditionally. >Ha, I fixed this accidentally in my patchset. :)> There''s ample room for further cleanup here, but this keeps the fix simple. > > Signed-off-by: Sage Weil <sage@newdream.net>Reviewed-by: Li Zefan <lizf@cn.fujitsu.com> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Excerpts from Li Zefan''s message of 2010-12-09 21:11:25 -0500:> Sage Weil wrote: > > We were incorrectly taking the async path even for the sync ioctls by > > passing in &transid unconditionally. > > > > Ha, I fixed this accidentally in my patchset. :)Thanks guys, Unless either of you object, I''ll take this and the snapshot ABI change for the next rc. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 9 Dec 2010, Chris Mason wrote:> Excerpts from Li Zefan''s message of 2010-12-09 21:11:25 -0500: > > Sage Weil wrote: > > > We were incorrectly taking the async path even for the sync ioctls by > > > passing in &transid unconditionally. > > > > > > > Ha, I fixed this accidentally in my patchset. :) > > Thanks guys, > > Unless either of you object, I''ll take this and the snapshot ABI change > for the next rc.Sounds good to me. Just FYI, I''m still seeing some weird corruption and crashes on my machines and haven''t narrowed it down yet. Should have more info soon. sage -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html