From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from userp1040.oracle.com ([156.151.31.81]:23887 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755964AbaEPOI7 (ORCPT ); Fri, 16 May 2014 10:08:59 -0400 Message-ID: <53761C9F.5050005@oracle.com> Date: Fri, 16 May 2014 22:11:43 +0800 From: Anand Jain MIME-Version: 1.0 To: Wang Shilong CC: linux-btrfs@vger.kernel.org Subject: Re: [PATCH 2/3 RFC] btrfs: total_devices should count replacing devices References: <1394207319-13252-1-git-send-email-Anand.Jain@oracle.com> <1394207319-13252-2-git-send-email-Anand.Jain@oracle.com> <5371E346.3000704@cn.fujitsu.com> <537329B9.6010605@oracle.com> In-Reply-To: <537329B9.6010605@oracle.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Sender: linux-btrfs-owner@vger.kernel.org List-ID: Wang, After a much of investigation on this, I think I found a better approach to fix this. Can you kindly comment on patch [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots Thanks, Anand On 14/05/14 16:30, Anand Jain wrote: > > Hello Wang, > > sure will do. Thanks for the comments. > > Anand > > On 13/05/14 17:17, Wang Shilong wrote: >> Hello Anand, >> >> I agree we can export @total_devices to fix 'btrfs file show' problem. >> This patch addressed two problem, it is better to split it into two >> patches. >> >> Could you please resend the patch, at least we should fix 'btrfs file >> show' >> problem firstly. >> >> Thanks, >> Wang >> >> On 03/07/2014 11:48 PM, Anand Jain wrote: >>> ioctl(BTRFS_IOC_FS_INFO) returns num_devices which does not >>> count seed device. num_devices is used to calculate >>> the number of slots during the ioctl(BTRFS_IOC_DEV_INFO) >>> but ioctl(BTRFS_IOC_DEV_INFO) would count seed devices as well. >>> Due to this miss match btrfs_progs get_fs_info() hits the bug.. >>> get_fs_info() >>> :: >>> BUG_ON(ndevs >= fi_args->num_devices); >>> >>> what I notice is total_devices should be returned by the >>> ioctl(BTRFS_IOC_FS_INFO)), however total_devices does not >>> count (transient) replacing device but num_devices does. >>> >>> first, num_devices which appears to be a subset of >>> total_devices helps to count the number of devices >>> under a FSID which excludes the seed devices but >>> includes the transient replacing device. >>> >>> total_devices which includes seed devices is as of >>> now does not count the tansient replacing device, >>> which IMO is a bug or the implementation details >>> are not clear enough to state the role of num_devices >>> and total_devices. >>> >>> The user land on the otherhand would want to know >>> all the devices that would come out of probe >>> against a FSID >>> >>> As of now in this patch I am making the ioctl.c local >>> changes (instead of asking replace thread to update >>> total_devices) to include replacing device. >>> >>> Signed-off-by: Anand Jain >>> --- >>> fs/btrfs/ioctl.c | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c >>> index 5036f9d..ec7d5c6 100644 >>> --- a/fs/btrfs/ioctl.c >>> +++ b/fs/btrfs/ioctl.c >>> @@ -2510,6 +2510,7 @@ static long btrfs_ioctl_fs_info(struct >>> btrfs_root *root, void __user *arg) >>> struct btrfs_device *next; >>> struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices; >>> int ret = 0; >>> + int dev_replace_running = 0; >>> if (!capable(CAP_SYS_ADMIN)) >>> return -EPERM; >>> @@ -2518,8 +2519,18 @@ static long btrfs_ioctl_fs_info(struct >>> btrfs_root *root, void __user *arg) >>> if (!fi_args) >>> return -ENOMEM; >>> + btrfs_dev_replace_lock(&root->fs_info->dev_replace); >>> + if (btrfs_dev_replace_is_ongoing(&root->fs_info->dev_replace)) >>> + dev_replace_running = 1; >>> + >>> + btrfs_dev_replace_unlock(&root->fs_info->dev_replace); >>> + >>> mutex_lock(&fs_devices->device_list_mutex); >>> - fi_args->num_devices = fs_devices->num_devices; >>> + if (dev_replace_running) >>> + fi_args->num_devices = fs_devices->total_devices + 1; >>> + else >>> + fi_args->num_devices = fs_devices->total_devices; >>> + >>> memcpy(&fi_args->fsid, root->fs_info->fsid, >>> sizeof(fi_args->fsid)); >>> list_for_each_entry_safe(device, next, &fs_devices->devices, >>> dev_list) { >> >> -- >> 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