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
