libblkid: Idea to force given cached entry to be invalidated?

libblkid: Idea to force given cached entry to be invalidated?

[Qu Wenruo]

Hi,

When using the blkid_get_cache(), things will be cached to reduce the 
probe frequency.

But filesystem which supports device management like btrfs can remove 
device and
if fast enough, libblkid will not invalidate the old cache and cause to 
get the wrong filesystem type.

The bug is describe in the following btrfs patch:
https://patchwork.kernel.org/patch/3960191/

Though the above patch try to avoid the whole cache, it's not good 
enough since it break the pricible not
to probe the devices too frequently.

After some searching, it seems that libblkid does not provide such API, 
or am I missing something?

So is there any idea about forcing given cached entry invalidated?

Thanks,
Qu

Re: libblkid: Idea to force given cached entry to be invalidated?

[Theodore Ts'o]

On Tue, Apr 15, 2014 at 10:15:01AM +0800, Qu Wenruo wrote:
> 
> Though the above patch try to avoid the whole cache, it's not good enough
> since it break the pricible not
> to probe the devices too frequently.
> 
> After some searching, it seems that libblkid does not provide such API, or
> am I missing something?

The blkid_verify() function is designed to make sure the information
in the cache corresponds with reality.  If it does not, it will update
the cache to match with reality, so it will do what you want.

Using blkid_verify() will involve accessing the disk involved, but
that's still better than the alternative of scanning all of the disks
in the system.

Cheers,

					- Ted

Re: libblkid: Idea to force given cached entry to be invalidated?

[Qu Wenruo]

-------- Original Message --------
Subject: Re: libblkid: Idea to force given cached entry to be invalidated?
From: Theodore Ts'o <tytso@mit.edu>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014年04月15日 12:27
> On Tue, Apr 15, 2014 at 10:15:01AM +0800, Qu Wenruo wrote:
>> Though the above patch try to avoid the whole cache, it's not good enough
>> since it break the pricible not
>> to probe the devices too frequently.
>>
>> After some searching, it seems that libblkid does not provide such API, or
>> am I missing something?
> The blkid_verify() function is designed to make sure the information
> in the cache corresponds with reality.  If it does not, it will update
> the cache to match with reality, so it will do what you want.
>
> Using blkid_verify() will involve accessing the disk involved, but
> that's still better than the alternative of scanning all of the disks
> in the system.
>
> Cheers,
>
> 					- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
I hope I was wrong but according to the codes, blkid_verify() seems 
still using the cache:
------
blkid_dev blkid_verify(blkid_cache cache, blkid_dev dev)
{
......
             (diff < BLKID_PROBE_MIN ||
                 (dev->bid_flags & BLKID_BID_FL_VERIFIED &&
                  diff < BLKID_PROBE_INTERVAL)))
                 return dev;
------

So it seems not working in the case.

Also in the btrfs-progs codes, it has already called blkid_verify() 
before operations.

Thanks,
Qu.

Re: libblkid: Idea to force given cached entry to be invalidated?

[Karel Zak]

On Tue, Apr 15, 2014 at 10:15:01AM +0800, Qu Wenruo wrote:
> When using the blkid_get_cache(), things will be cached to reduce the probe
> frequency.

The question is if you really want to use the cache at all :-) 

It's pretty common that we have udevd that maintain all necessary
information about devices, including information from libblkid. The
udev db is the primary source for many system actions and tools.

All you need is to use libudev udev_enumerate API to scan for all
block devices and udev_device_get_property_value(ID_FS_TYPE). Maybe
you can add a new method BTRFS_SCAN_UDEV and use it as default. It
would be better to keep libblkid as a fallback solution only. (For
very unusual situations where there is no udev or so.)

> But filesystem which supports device management like btrfs can remove device
> and if fast enough, libblkid will not invalidate the old cache and cause to get
> the wrong filesystem type.

Well, since commit 6c2f2b9d (year 2010, v2.18), the libblkid uses
microsecond resolution to check for device modifications.

So the problem is not that btrfs is fast enough ;-)

> The bug is describe in the following btrfs patch:
> https://patchwork.kernel.org/patch/3960191/

The patch does not make sense at all, blkid without a cache scans
/proc/partitions -- so it's almost the same like BTRFS_SCAN_PROC
btrfs_scan_block_devices().


> So is there any idea about forcing given cached entry invalidated?

It seems you're trying workaround the *btrfs problem*. The command 

  btrfs device delete

does not open and modify the device! It uses btrfs ioctl and kernel
does the changes to the device. The problem is that kernel does not
update atime and mtime of the device. So the changes are completely
invisible for libblkid (or another mtime based solutions). See
my example below.

IMHO it's strange manner to modify files in kernel, but no update 
mtime.

Fortunately kernel at least sends event to udev.

Maybe possible workaround is to add utime() call to "btrfs device
delete". Something like:

diff --git a/cmds-device.c b/cmds-device.c
index 309b49e..5c438bf 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -20,6 +20,8 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/ioctl.h>
+#include <sys/types.h>
+#include <utime.h>
 #include <errno.h>
 #include <sys/stat.h>
 #include <getopt.h>
@@ -179,7 +181,8 @@ static int cmd_rm_dev(int argc, char **argv)
 				"ERROR: error removing the device '%s' - %s\n",
 				argv[i], strerror(e));
 			ret++;
-		}
+		} else
+			utime(argv[i], NULL);
 	}
 
 	close_file_or_dir(fdmnt, dirstream);


BTW, it would be nice to have --verbose option for btrfs-scan, now it
seems like black box.

    Karel


See mtime:


# stat /dev/sdb2
  File: ‘/dev/sdb2’
  Size: 0               Blocks: 0          IO Block: 4096   block special file
Device: 5h/5d   Inode: 392701      Links: 1     Device type: 8,12
Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/    disk)
Access: 2014-04-15 11:58:58.338758155 +0200
Modify: 2014-04-15 11:58:58.338758155 +0200
Change: 2014-04-15 11:58:58.338758155 +0200
 Birth: -

# btrfs device add /dev/sdb2 /mnt/test
SMALL VOLUME: forcing mixed metadata/data groups

# stat /dev/sdb2
  File: ‘/dev/sdb2’
  Size: 0               Blocks: 0          IO Block: 4096   block special file
Device: 5h/5d   Inode: 392701      Links: 1     Device type: 8,12
Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/    disk)
Access: 2014-04-15 11:59:34.930143021 +0200
Modify: 2014-04-15 11:59:34.930143021 +0200
Change: 2014-04-15 11:59:34.930143021 +0200
                            ^^^^^^^^

# btrfs device delete /dev/sdb2 /mnt/test

# stat /dev/sdb2
  File: ‘/dev/sdb2’
  Size: 0               Blocks: 0          IO Block: 4096   block special file
Device: 5h/5d   Inode: 392701      Links: 1     Device type: 8,12
Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/    disk)
Access: 2014-04-15 11:59:34.930143021 +0200
Modify: 2014-04-15 11:59:34.930143021 +0200
Change: 2014-04-15 11:59:34.930143021 +0200
                            ^^^^^^^^^


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: libblkid: Idea to force given cached entry to be invalidated?

[Qu Wenruo]

-------- Original Message --------
Subject: Re: libblkid: Idea to force given cached entry to be invalidated?
From: Karel Zak <kzak@redhat.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014=E5=B9=B404=E6=9C=8815=E6=97=A5 19:21
> On Tue, Apr 15, 2014 at 10:15:01AM +0800, Qu Wenruo wrote:
>> When using the blkid_get_cache(), things will be cached to reduce the pr=
obe
>> frequency.
> The question is if you really want to use the cache at all :-)
>
> It's pretty common that we have udevd that maintain all necessary
> information about devices, including information from libblkid. The
> udev db is the primary source for many system actions and tools.
>
> All you need is to use libudev udev_enumerate API to scan for all
> block devices and udev_device_get_property_value(ID_FS_TYPE). Maybe
> you can add a new method BTRFS_SCAN_UDEV and use it as default. It
> would be better to keep libblkid as a fallback solution only. (For
> very unusual situations where there is no udev or so.)
Thanks for the suggestion to use libudev.
It sounds solid and I'll try to use it instead.
>
>> But filesystem which supports device management like btrfs can remove de=
vice
>> and if fast enough, libblkid will not invalidate the old cache and cause=
 to get
>> the wrong filesystem type.
> Well, since commit 6c2f2b9d (year 2010, v2.18), the libblkid uses
> microsecond resolution to check for device modifications.
>
> So the problem is not that btrfs is fast enough ;-)
According to your explaination, it seems that if there is any=20
modification to the block device, libblkid will rescan it and not
to use the old cache, do I understand it right?
>
>> The bug is describe in the following btrfs patch:
>> https://patchwork.kernel.org/patch/3960191/
> The patch does not make sense at all, blkid without a cache scans
> /proc/partitions -- so it's almost the same like BTRFS_SCAN_PROC
> btrfs_scan_block_devices().
>
>
>> So is there any idea about forcing given cached entry invalidated?
> It seems you're trying workaround the *btrfs problem*. The command
>
>    btrfs device delete
>
> does not open and modify the device! It uses btrfs ioctl and kernel
> does the changes to the device. The problem is that kernel does not
> update atime and mtime of the device. So the changes are completely
> invisible for libblkid (or another mtime based solutions). See
> my example below.
Thanks for pointing out the problem a lot !

>
> IMHO it's strange manner to modify files in kernel, but no update
> mtime.
>
> Fortunately kernel at least sends event to udev.
>
> Maybe possible workaround is to add utime() call to "btrfs device
> delete". Something like:
This sounds good, much better than my poor patch.
I will also try to fix it in kernel codes if possible.
>
> diff --git a/cmds-device.c b/cmds-device.c
> index 309b49e..5c438bf 100644
> --- a/cmds-device.c
> +++ b/cmds-device.c
> @@ -20,6 +20,8 @@
>   #include <unistd.h>
>   #include <fcntl.h>
>   #include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <utime.h>
>   #include <errno.h>
>   #include <sys/stat.h>
>   #include <getopt.h>
> @@ -179,7 +181,8 @@ static int cmd_rm_dev(int argc, char **argv)
>   				"ERROR: error removing the device '%s' - %s\n",
>   				argv[i], strerror(e));
>   			ret++;
> -		}
> +		} else
> +			utime(argv[i], NULL);
>   	}
>  =20
>   	close_file_or_dir(fdmnt, dirstream);
>
>
> BTW, it would be nice to have --verbose option for btrfs-scan, now it
> seems like black box.
I'd like to add it, but as you know, 'btrfs dev scan' just calls a btrfs=20
ioctl, so even '--verbose' is added,
nothing useful will be printed out. :(

>
>      Karel
>
>
> See mtime:
>
>
> # stat /dev/sdb2
>    File: =E2=80=98/dev/sdb2=E2=80=99
>    Size: 0               Blocks: 0          IO Block: 4096   block specia=
l file
> Device: 5h/5d   Inode: 392701      Links: 1     Device type: 8,12
> Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/    disk)
> Access: 2014-04-15 11:58:58.338758155 +0200
> Modify: 2014-04-15 11:58:58.338758155 +0200
> Change: 2014-04-15 11:58:58.338758155 +0200
>   Birth: -
>
> # btrfs device add /dev/sdb2 /mnt/test
> SMALL VOLUME: forcing mixed metadata/data groups
>
> # stat /dev/sdb2
>    File: =E2=80=98/dev/sdb2=E2=80=99
>    Size: 0               Blocks: 0          IO Block: 4096   block specia=
l file
> Device: 5h/5d   Inode: 392701      Links: 1     Device type: 8,12
> Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/    disk)
> Access: 2014-04-15 11:59:34.930143021 +0200
> Modify: 2014-04-15 11:59:34.930143021 +0200
> Change: 2014-04-15 11:59:34.930143021 +0200
>                              ^^^^^^^^
>
> # btrfs device delete /dev/sdb2 /mnt/test
>
> # stat /dev/sdb2
>    File: =E2=80=98/dev/sdb2=E2=80=99
>    Size: 0               Blocks: 0          IO Block: 4096   block specia=
l file
> Device: 5h/5d   Inode: 392701      Links: 1     Device type: 8,12
> Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/    disk)
> Access: 2014-04-15 11:59:34.930143021 +0200
> Modify: 2014-04-15 11:59:34.930143021 +0200
> Change: 2014-04-15 11:59:34.930143021 +0200
>                              ^^^^^^^^^
>
>
Great thanks for all your advise! It helps a lot!!

Thanks,
Qu

Re: libblkid: Idea to force given cached entry to be invalidated?

[Karel Zak]

On Wed, Apr 16, 2014 at 08:53:27AM +0800, Qu Wenruo wrote:
> >So the problem is not that btrfs is fast enough ;-)
> According to your explaination, it seems that if there is any modification
> to the block device, libblkid will rescan it and not
> to use the old cache, do I understand it right?

Yes, blkid_verify():

	if (now >= dev->bid_time &&
#ifdef HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC
	    (st.st_mtime < dev->bid_time ||
	        (st.st_mtime == dev->bid_time &&
		 st.st_mtim.tv_nsec / 1000 <= dev->bid_utime)) &&
#else
	    st.st_mtime <= dev->bid_time &&
#endif
	    (diff < BLKID_PROBE_MIN ||
		(dev->bid_flags & BLKID_BID_FL_VERIFIED &&
		 diff < BLKID_PROBE_INTERVAL)))
		return dev;


It checks two basic things:

 a) compare device st_mtime with cache entry time (bid_time and bid_utime), 
    the valid cache entry has not to be older than device mtime.

 b) valid cache entry has not to be older than BLKID_PROBE_MIN (2 sec)
    and previous blkid_verify() validation remaining in memory has not 
    to be older than BLKID_PROBE_INTERVAL (200s). 

> >BTW, it would be nice to have --verbose option for btrfs-scan, now it
> >seems like black box.
> I'd like to add it, but as you know, 'btrfs dev scan' just calls a btrfs
> ioctl, so even '--verbose' is added,
> nothing useful will be printed out. :(

Well, I mean at least info about for what devices it calls the ioctls.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: libblkid: Idea to force given cached entry to be invalidated?

[Qu Wenruo]

-------- Original Message --------
Subject: Re: libblkid: Idea to force given cached entry to be invalidated?
From: Karel Zak <kzak@redhat.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014年04月16日 17:03
> On Wed, Apr 16, 2014 at 08:53:27AM +0800, Qu Wenruo wrote:
>>> So the problem is not that btrfs is fast enough ;-)
>> According to your explaination, it seems that if there is any modification
>> to the block device, libblkid will rescan it and not
>> to use the old cache, do I understand it right?
> Yes, blkid_verify():
>
> 	if (now >= dev->bid_time &&
> #ifdef HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC
> 	    (st.st_mtime < dev->bid_time ||
> 	        (st.st_mtime == dev->bid_time &&
> 		 st.st_mtim.tv_nsec / 1000 <= dev->bid_utime)) &&
> #else
> 	    st.st_mtime <= dev->bid_time &&
> #endif
> 	    (diff < BLKID_PROBE_MIN ||
> 		(dev->bid_flags & BLKID_BID_FL_VERIFIED &&
> 		 diff < BLKID_PROBE_INTERVAL)))
> 		return dev;
>
>
> It checks two basic things:
>
>   a) compare device st_mtime with cache entry time (bid_time and bid_utime),
>      the valid cache entry has not to be older than device mtime.
>
>   b) valid cache entry has not to be older than BLKID_PROBE_MIN (2 sec)
>      and previous blkid_verify() validation remaining in memory has not
>      to be older than BLKID_PROBE_INTERVAL (200s).
>
>>> BTW, it would be nice to have --verbose option for btrfs-scan, now it
>>> seems like black box.
>> I'd like to add it, but as you know, 'btrfs dev scan' just calls a btrfs
>> ioctl, so even '--verbose' is added,
>> nothing useful will be printed out. :(
> Well, I mean at least info about for what devices it calls the ioctls.
>
>      Karel
>
Thanks for all your explaination.
Now I think I am somewhat clear of the libblkid cache now.

But after some tests, it seems that the ctime/mtime based cache seems 
not perfect due to the fact that multiple
device file can be created in different fs with same major/minor number.
especially each of them has individual ctime/mtime timestamp.

So if someone (maybe insane) use mknod to create as block device file 
and use 'btrfs dev del' on the newly created
block device file, libblkid will still be unable to detect the change 
since the ctime/mtime of block device file in /dev/
does not change.

Will libblkid be able to deal with it in later versions? (Although not 
related to my original problem now)

Thanks,
Qu

Re: libblkid: Idea to force given cached entry to be invalidated?

[Karel Zak]

On Thu, Apr 17, 2014 at 09:17:03AM +0800, Qu Wenruo wrote:
> But after some tests, it seems that the ctime/mtime based cache seems not
> perfect due to the fact that multiple

Yes, the cache is not perfect -- it was originally introduced by Ted
in time when things was less complicated :-) The current goal is to
avoid the cache usage, so we have low-level API in libblkid to bypass
the cache at all, and in userspace we use this API or udev db. I guess
that many users have the cache empty, because it's unnecessary for
fsck, mount, lsblk, findmnt, systemd etc.

> device file can be created in different fs with same major/minor number.
> especially each of them has individual ctime/mtime timestamp.
> 
> So if someone (maybe insane) use mknod to create as block device file and
> use 'btrfs dev del' on the newly created
> block device file, libblkid will still be unable to detect the change since
> the ctime/mtime of block device file in /dev/
> does not change.

Not sure if I understand, it would be nice to have complete example.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: libblkid: Idea to force given cached entry to be invalidated?

[Qu Wenruo]

-------- Original Message --------
Subject: Re: libblkid: Idea to force given cached entry to be invalidated?
From: Karel Zak <kzak@redhat.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014=E5=B9=B404=E6=9C=8817=E6=97=A5 16:21
> On Thu, Apr 17, 2014 at 09:17:03AM +0800, Qu Wenruo wrote:
>> But after some tests, it seems that the ctime/mtime based cache seems no=
t
>> perfect due to the fact that multiple
> Yes, the cache is not perfect -- it was originally introduced by Ted
> in time when things was less complicated :-) The current goal is to
> avoid the cache usage, so we have low-level API in libblkid to bypass
> the cache at all, and in userspace we use this API or udev db. I guess
> that many users have the cache empty, because it's unnecessary for
> fsck, mount, lsblk, findmnt, systemd etc.
>
>> device file can be created in different fs with same major/minor number.
>> especially each of them has individual ctime/mtime timestamp.
>>
>> So if someone (maybe insane) use mknod to create as block device file an=
d
>> use 'btrfs dev del' on the newly created
>> block device file, libblkid will still be unable to detect the change si=
nce
>> the ctime/mtime of block device file in /dev/
>> does not change.
> Not sure if I understand, it would be nice to have complete example.
>
>      Karel
>
Sorry for my poor English.
Use /dev/sda6(major 8, minor 6) as an example.
------
# stat  /dev/sda6
   File: '/dev/sda6'
   Size: 0             Blocks: 0          IO Block: 4096   block special=20
file
Device: 5h/5d    Inode: 7289        Links: 1     Device type: 8,6
Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/ disk)
Access: 2014-04-17 12:30:08.981732698 +0800
Modify: 2014-04-17 12:30:08.981732698 +0800
Change: 2014-04-17 12:30:08.981732698 +0800
                                      ^^^^^^^^^^^^^^^^^
  Birth: -
# mknod sda6 b 8 6
# mkfs.ext4 ./sda6 <<< Not /dev/sda6
......
# stat  /dev/sda6
   File: '/dev/sda6'
   Size: 0             Blocks: 0          IO Block: 4096   block special=20
file
Device: 5h/5d    Inode: 7289        Links: 1     Device type: 8,6
Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/ disk)
Access: 2014-04-17 12:30:08.981732698 +0800
Modify: 2014-04-17 12:30:08.981732698 +0800
Change: 2014-04-17 12:30:08.981732698 +0800
                                        ^^^^^^^^^^^^^^^^ unchanged
  Birth: -

# stat  ./sda6
   File: './sda6'
   Size: 0             Blocks: 0          IO Block: 4096   block special=20
file
Device: 805h/2053d    Inode: 8889848     Links: 1     Device type: 8,6
Access: (0644/brw-r--r--)  Uid: (    0/    root)   Gid: (    0/ root)
Access: 2014-04-17 16:26:30.312439395 +0800
Modify: 2014-04-17 16:26:40.869106587 +0800
Change: 2014-04-17 16:26:40.869106587 +0800  <<< Only the created block=20
device file is updated
  Birth: -
------

Thanks,
Qu

Re: libblkid: Idea to force given cached entry to be invalidated?

[Karel Zak]

On Thu, Apr 17, 2014 at 04:29:06PM +0800, Qu Wenruo wrote:
> >>So if someone (maybe insane) use mknod to create as block device file and
> >>use 'btrfs dev del' on the newly created
> >>block device file, libblkid will still be unable to detect the change since
> >>the ctime/mtime of block device file in /dev/
> >>does not change.
> >Not sure if I understand, it would be nice to have complete example.
> >
> >     Karel
> >
> Sorry for my poor English.
> Use /dev/sda6(major 8, minor 6) as an example.
> ------
> # stat  /dev/sda6
>   File: '/dev/sda6'
>   Size: 0             Blocks: 0          IO Block: 4096   block special file
> Device: 5h/5d    Inode: 7289        Links: 1     Device type: 8,6
> Access: (0660/brw-rw----)  Uid: (    0/    root)   Gid: (    6/ disk)
> Access: 2014-04-17 12:30:08.981732698 +0800
> Modify: 2014-04-17 12:30:08.981732698 +0800
> Change: 2014-04-17 12:30:08.981732698 +0800
>                                      ^^^^^^^^^^^^^^^^^
>  Birth: -
> # mknod sda6 b 8 6
> # mkfs.ext4 ./sda6 <<< Not /dev/sda6

 Ah... it's too crazy to support this scenario :-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com