Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove.
Original Message Subject: Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove. From: David Sterba dste...@suse.cz To: Qu Wenruo quwen...@cn.fujitsu.com Date: 2014年05月29日 20:43 On Wed, Apr 16, 2014 at 05:02:32PM +0800, Qu Wenruo wrote: @@ -1704,10 +1720,14 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path) ret = 0; - /* Notify udev that device has changed */ - if (bdev) + if (bdev) { + /* Notify udev that device has changed */ btrfs_kobject_uevent(bdev, KOBJ_CHANGE); + /* Update ctime/mtime for device path for libblkid */ + update_dev_time(device_path); The change on the device comes after the uevent notification, is it possible that the event is delivered and processed before the device times are updated? I would say so. If I understand the udev thing right, uevent notification is sent to udevd and udevd process the notification. For btrfs_rm_device(), it will send KOBJ_CHANGE uevent to udevd, and udevd will *update* the ctime/mtime in /dev. But the problems is, there is a time lag between uevent sending and udevd updating the ctime/mtime, which makes the ctime/mtime updating later than next 'btrfs dev scan' commands. Since the ctime/mtime is not changed, libblkid used by 'btrfs dev scan' will still consider the removed disk as btrfs, causing the bug I reported. IMO the best method to deal the bug is to wait udevd processing the uevent, but unfortunately, we can't even guarantee that there is a udevd processing the uevent(like some embedded system without udevd). So I use update_dev_time() as a workaround, which provide a completely synchronize method to update device file ctime/mtime. And even the uevent is processed before the update_dev_time(), it makes no harm except a new ctime/mtime. Thanks, Qu + } + error_brelse: brelse(bh); if (bdev) -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove.
Hi Qu, In line below... On 16/04/14 17:02, Qu Wenruo wrote: Btrfs will send uevent to udev inform the device change, but ctime/mtime for the block device inode is not udpated, which cause libblkid used by btrfs-progs unable to detect device change and use old cache, causing 'btrfs dev scan; btrfs dev rmove; btrfs dev scan' give an error message. Reported-by: Tsutomu Itoh t-i...@jp.fujitsu.com Cc: Karel Zak k...@redhat.com Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com --- fs/btrfs/volumes.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 49d7fab..ce232d7 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1452,6 +1452,22 @@ out: return ret; } +/* + * Function to update ctime/mtime for a given device path. + * Mainly used for ctime/mtime based probe like libblkid. + */ +static void update_dev_time(char *path_name) +{ + struct file *filp; + + filp = filp_open(path_name, O_RDWR, 0); + if (!filp) + return; + file_update_time(filp); + filp_close(filp, NULL); + return; +} + IMO /(I might be wrong) I think its not a good idea to explicitly achieve this. Since this thread would have already scratched the device's SB, shouldn't that take care of updating the mtime ? Thanks, Anand static int btrfs_rm_dev_item(struct btrfs_root *root, struct btrfs_device *device) { @@ -1704,10 +1720,14 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path) ret = 0; - /* Notify udev that device has changed */ - if (bdev) + if (bdev) { + /* Notify udev that device has changed */ btrfs_kobject_uevent(bdev, KOBJ_CHANGE); + /* Update ctime/mtime for device path for libblkid */ + update_dev_time(device_path); + } + error_brelse: brelse(bh); if (bdev) @@ -2146,6 +2166,8 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path) ret = btrfs_commit_transaction(trans, root); } + /* Update ctime/mtime for libblkid */ + update_dev_time(device_path); return ret; error_trans: -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove.
Original Message Subject: Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove. From: Anand Jain anand.j...@oracle.com To: Qu Wenruo quwen...@cn.fujitsu.com, linux-btrfs@vger.kernel.org Date: 2014年05月30日 15:51 Hi Qu, In line below... On 16/04/14 17:02, Qu Wenruo wrote: Btrfs will send uevent to udev inform the device change, but ctime/mtime for the block device inode is not udpated, which cause libblkid used by btrfs-progs unable to detect device change and use old cache, causing 'btrfs dev scan; btrfs dev rmove; btrfs dev scan' give an error message. Reported-by: Tsutomu Itoh t-i...@jp.fujitsu.com Cc: Karel Zak k...@redhat.com Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com --- fs/btrfs/volumes.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 49d7fab..ce232d7 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1452,6 +1452,22 @@ out: return ret; } +/* + * Function to update ctime/mtime for a given device path. + * Mainly used for ctime/mtime based probe like libblkid. + */ +static void update_dev_time(char *path_name) +{ +struct file *filp; + +filp = filp_open(path_name, O_RDWR, 0); +if (!filp) +return; +file_update_time(filp); +filp_close(filp, NULL); +return; +} + IMO /(I might be wrong) I think its not a good idea to explicitly achieve this. Since this thread would have already scratched the device's SB, shouldn't that take care of updating the mtime ? Thanks, Anand Yes, scratching SB will update the time of the device's inode. But the problem is that, the *block file*'s ctime/mtime is not changed according to the device's inode mtime/ctime. So in the patch, I use the device's *full path* not the *device's inode* to update ctime/mtime since kernel operations will not update ctime/mtine of *device files under /dev*. About your fix patch ([PATCH] btrfs: kobject_uevent should use bd_part instead of bd_disk ) I am unable to reproduce the problem on 3.15-rc4 kernel with 3.14.1 btrfs-progs. :( But the ctime/mtime things will not change as the following example: -- # btrfs dev scan;stat /dev/sdd ; btrfs dev del /dev/sdd /mnt/scratch/;stat /dev/sdd ; btrfs dev scan Scanning for Btrfs filesystems File: '/dev/sdd' Size: 0 Blocks: 0 IO Block: 4096 block special file Device: 5h/5dInode: 8569Links: 1 Device type: 8,30 Access: (0660/brw-rw) Uid: (0/root) Gid: (6/ disk) Access: 2014-05-30 16:39:06.104006687 +0800 Modify: 2014-05-30 16:38:57.638006272 +0800 ctime/mtime does not change Change: 2014-05-30 16:38:57.638006272 +0800 Birth: - File: '/dev/sdd' Size: 0 Blocks: 0 IO Block: 4096 block special file Device: 5h/5dInode: 8569Links: 1 Device type: 8,30 Access: (0660/brw-rw) Uid: (0/root) Gid: (6/ disk) Access: 2014-05-30 16:39:06.104006687 +0800 Modify: 2014-05-30 16:38:57.638006272 +0800 ctime/mtime does not change Change: 2014-05-30 16:38:57.638006272 +0800 Birth: - Scanning for Btrfs filesystems ^^^ But no error message now :( -- Thanks, Qu static int btrfs_rm_dev_item(struct btrfs_root *root, struct btrfs_device *device) { @@ -1704,10 +1720,14 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path) ret = 0; -/* Notify udev that device has changed */ -if (bdev) +if (bdev) { +/* Notify udev that device has changed */ btrfs_kobject_uevent(bdev, KOBJ_CHANGE); +/* Update ctime/mtime for device path for libblkid */ +update_dev_time(device_path); +} + error_brelse: brelse(bh); if (bdev) @@ -2146,6 +2166,8 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path) ret = btrfs_commit_transaction(trans, root); } +/* Update ctime/mtime for libblkid */ +update_dev_time(device_path); return ret; error_trans: -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove.
On Wed, Apr 16, 2014 at 05:02:32PM +0800, Qu Wenruo wrote: @@ -1704,10 +1720,14 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path) ret = 0; - /* Notify udev that device has changed */ - if (bdev) + if (bdev) { + /* Notify udev that device has changed */ btrfs_kobject_uevent(bdev, KOBJ_CHANGE); + /* Update ctime/mtime for device path for libblkid */ + update_dev_time(device_path); The change on the device comes after the uevent notification, is it possible that the event is delivered and processed before the device times are updated? I would say so. + } + error_brelse: brelse(bh); if (bdev) -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove.
Original Message Subject: Re: [PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove. From: David Sterba dste...@suse.cz To: Qu Wenruo quwen...@cn.fujitsu.com Date: 2014年05月29日 20:43 On Wed, Apr 16, 2014 at 05:02:32PM +0800, Qu Wenruo wrote: @@ -1704,10 +1720,14 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path) ret = 0; - /* Notify udev that device has changed */ - if (bdev) + if (bdev) { + /* Notify udev that device has changed */ btrfs_kobject_uevent(bdev, KOBJ_CHANGE); + /* Update ctime/mtime for device path for libblkid */ + update_dev_time(device_path); The change on the device comes after the uevent notification, is it possible that the event is delivered and processed before the device times are updated? I would say so. Yes, udev event is delivered before device times updated, but now btrfs-progs still use libblkid to do device scan things as default, so I didn't catch the point. Would you please tell me what the problem is? Thanks, Qu + } + error_brelse: brelse(bh); if (bdev) -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC] btrfs: Add ctime/mtime update for btrfs device add/remove.
Btrfs will send uevent to udev inform the device change, but ctime/mtime for the block device inode is not udpated, which cause libblkid used by btrfs-progs unable to detect device change and use old cache, causing 'btrfs dev scan; btrfs dev rmove; btrfs dev scan' give an error message. Reported-by: Tsutomu Itoh t-i...@jp.fujitsu.com Cc: Karel Zak k...@redhat.com Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com --- fs/btrfs/volumes.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 49d7fab..ce232d7 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1452,6 +1452,22 @@ out: return ret; } +/* + * Function to update ctime/mtime for a given device path. + * Mainly used for ctime/mtime based probe like libblkid. + */ +static void update_dev_time(char *path_name) +{ + struct file *filp; + + filp = filp_open(path_name, O_RDWR, 0); + if (!filp) + return; + file_update_time(filp); + filp_close(filp, NULL); + return; +} + static int btrfs_rm_dev_item(struct btrfs_root *root, struct btrfs_device *device) { @@ -1704,10 +1720,14 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path) ret = 0; - /* Notify udev that device has changed */ - if (bdev) + if (bdev) { + /* Notify udev that device has changed */ btrfs_kobject_uevent(bdev, KOBJ_CHANGE); + /* Update ctime/mtime for device path for libblkid */ + update_dev_time(device_path); + } + error_brelse: brelse(bh); if (bdev) @@ -2146,6 +2166,8 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path) ret = btrfs_commit_transaction(trans, root); } + /* Update ctime/mtime for libblkid */ + update_dev_time(device_path); return ret; error_trans: -- 1.9.2 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html