Re: [PATCH 3/3] btrfs: increase output size for LOGICAL_INO_V2 ioctl
Hi, On Sat, Sep 23, 2017 at 11:06:42PM +0200, Hans van Kranenburg wrote: > Reviewed-by: Hans van Kranenburg > Tested-by: Hans van Kranenburg the patches look good to me and the usecase and testing coverage seem sufficient to take the patches to 4.15, though we're close to the informal merging deadline. -- 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 3/3] btrfs: increase output size for LOGICAL_INO_V2 ioctl
On 09/22/2017 07:58 PM, Zygo Blaxell wrote:
> Build-server workloads have hundreds of references per file after dedup.
> Multiply by a few snapshots and we quickly exhaust the limit of 2730
> references per extent that can fit into a 64K buffer.
Simulating this scenario:
/btrfs 2-# btrfs sub create 0
Create subvolume './0'
/btrfs 2-# cp /boot/vmlinuz-4.14.0-rc1-zygo1 0/0
/btrfs 2-# for i in $(seq 1 499); do cp --reflink 0/0 0/$i; done
/btrfs 2-# for i in $(seq 1 5); do btrfs sub snap 0 $i; done
Create a snapshot of '0' in './1'
Create a snapshot of '0' in './2'
Create a snapshot of '0' in './3'
Create a snapshot of '0' in './4'
Create a snapshot of '0' in './5'
-# ./show_block_groups.py /btrfs
block group vaddr 0 length 4194304 flags SYSTEM used 16384 used_pct 0
block group vaddr 4194304 length 8388608 flags METADATA used 507904
used_pct 6
block group vaddr 12582912 length 8388608 flags DATA used 4198400
used_pct 50
block group vaddr 20971520 length 268435456 flags METADATA used 0 used_pct 0
-# ./show_block_group_contents.py 12582912 /btrfs
block group vaddr 12582912 length 8388608 flags DATA used 4198400
used_pct 50
extent vaddr 12582912 length 4198400 refs 500 gen 25 flags DATA
inline extent data backref root 257 objectid 262 offset 0 count 1
inline extent data backref root 257 objectid 277 offset 0 count 1
inline extent data backref root 257 objectid 288 offset 0 count 1
[...]
extent data backref root 257 objectid 663 offset 0 count 1
extent data backref root 257 objectid 366 offset 0 count 1
extent data backref root 257 objectid 715 offset 0 count 1
extent data backref root 257 objectid 306 offset 0 count 1
extent data backref root 257 objectid 470 offset 0 count 1
[...]
Total 500 lines, the extra 2500 files in the snapshots are hidden behind
the shared metadata refs now...
>
> Raise the limit to 16M to be consistent with other btrfs ioctls
> (e.g. TREE_SEARCH_V2, FILE_EXTENT_SAME).
>
> To minimize surprising userspace behavior, apply this change only to
> the LOGICAL_INO_V2 ioctl.
>
> Signed-off-by: Zygo Blaxell
> ---
> fs/btrfs/ioctl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index f4281ffd1833..1940678fc440 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4554,6 +4554,7 @@ static long btrfs_ioctl_logical_to_ino(struct
> btrfs_fs_info *fs_info,
>
> if (version == 1) {
> ignore_offset = false;
> + size = min_t(u32, loi->size, SZ_64K);
> } else {
> /* All reserved bits must be 0 for now */
> if (memchr_inv(loi->reserved, 0, sizeof(loi->reserved))) {
> @@ -4566,6 +4567,7 @@ static long btrfs_ioctl_logical_to_ino(struct
> btrfs_fs_info *fs_info,
> goto out_loi;
> }
> ignore_offset = loi->flags &
> BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET;
> + size = min_t(u32, loi->size, SZ_16M);
> }
>
> path = btrfs_alloc_path();
> @@ -4574,7 +4576,6 @@ static long btrfs_ioctl_logical_to_ino(struct
> btrfs_fs_info *fs_info,
> goto out;
> }
>
> - size = min_t(u32, loi->size, SZ_64K);
> inodes = init_data_container(size);
> if (IS_ERR(inodes)) {
> ret = PTR_ERR(inodes);
>
>>> import btrfs
>>> fs = btrfs.FileSystem('/btrfs')
Checking that 'v1' still works:
>>> inodes, bytes_missed = btrfs.ioctl.logical_to_ino(fs.fd, 12582912,
65536)
>>> len(inodes)
2730
>>> bytes_missed
6480
Yes, we only get 2730, as expected with a 64k buffer.
v2 can do the same:
>>> inodes, bytes_missed = btrfs.ioctl.logical_to_ino_v2(fs.fd,
12582912, 65536)
>>> len(inodes)
2730
>>> bytes_missed
6480
The bytes_missed is really useful, because it tells us the exact size of
the buf we need instead :)
>>> inodes, bytes_missed = btrfs.ioctl.logical_to_ino_v2(fs.fd,
12582912, 65536 + 6480)
>>> len(inodes)
3000
>>> bytes_missed
0
Yay!
If I remove the buffer size sanity check inside the python-btrfs ioctl code:
>>> import btrfs
>>> fs = btrfs.FileSystem('/btrfs')
>>> inodes, bytes_missed = btrfs.ioctl.logical_to_ino(fs.fd, 12582912,
65536 + 6480)
>>> len(inodes)
2730
>>> bytes_missed
6480
Yes, buffer still gets truncated to 64k in the v1 code.
Reviewed-by: Hans van Kranenburg
Tested-by: Hans van Kranenburg
--
Hans van Kranenburg
--
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 3/3] btrfs: increase output size for LOGICAL_INO_V2 ioctl
Build-server workloads have hundreds of references per file after dedup.
Multiply by a few snapshots and we quickly exhaust the limit of 2730
references per extent that can fit into a 64K buffer.
Raise the limit to 16M to be consistent with other btrfs ioctls
(e.g. TREE_SEARCH_V2, FILE_EXTENT_SAME).
To minimize surprising userspace behavior, apply this change only to
the LOGICAL_INO_V2 ioctl.
Signed-off-by: Zygo Blaxell
---
fs/btrfs/ioctl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index f4281ffd1833..1940678fc440 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4554,6 +4554,7 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
if (version == 1) {
ignore_offset = false;
+ size = min_t(u32, loi->size, SZ_64K);
} else {
/* All reserved bits must be 0 for now */
if (memchr_inv(loi->reserved, 0, sizeof(loi->reserved))) {
@@ -4566,6 +4567,7 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
goto out_loi;
}
ignore_offset = loi->flags &
BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET;
+ size = min_t(u32, loi->size, SZ_16M);
}
path = btrfs_alloc_path();
@@ -4574,7 +4576,6 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
goto out;
}
- size = min_t(u32, loi->size, SZ_64K);
inodes = init_data_container(size);
if (IS_ERR(inodes)) {
ret = PTR_ERR(inodes);
--
2.11.0
--
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 3/3] btrfs: increase output size for LOGICAL_INO_V2 ioctl
Build-server workloads have hundreds of references per file after dedup.
Multiply by a few snapshots and we quickly exhaust the limit of 2730
references per extent that can fit into a 64K buffer.
Raise the limit to 16M to be consistent with other btrfs ioctls
(e.g. TREE_SEARCH_V2, FILE_EXTENT_SAME).
To minimize surprising userspace behavior, apply this change only to
the LOGICAL_INO_V2 ioctl.
Signed-off-by: Zygo Blaxell
---
fs/btrfs/ioctl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 2bc3a9588d1d..4be9b1791f58 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4554,6 +4554,7 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
if (version == 1) {
ignore_offset = false;
+ size = min_t(u32, loi->size, SZ_64K);
} else {
/* Only accept flags we have defined so far */
if (loi->flags & ~(BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET)) {
@@ -4561,6 +4562,7 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
goto out_loi;
}
ignore_offset = loi->flags &
BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET;
+ size = min_t(u32, loi->size, SZ_16M);
}
path = btrfs_alloc_path();
@@ -4569,7 +4571,6 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
goto out;
}
- size = min_t(u32, loi->size, SZ_64K);
inodes = init_data_container(size);
if (IS_ERR(inodes)) {
ret = PTR_ERR(inodes);
--
2.11.0
--
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 3/3] btrfs: increase output size for LOGICAL_INO_V2 ioctl
Build-server workloads have hundreds of references per file after dedup.
Multiply by a few snapshots and we quickly exhaust the limit of 2730
references per extent that can fit into a 64K buffer.
Raise the limit to 16M to be consistent with other btrfs ioctls
(e.g. TREE_SEARCH_V2, FILE_EXTENT_SAME).
To minimize surprising userspace behavior, apply this change only to
the LOGICAL_INO_V2 ioctl.
Signed-off-by: Zygo Blaxell
---
fs/btrfs/ioctl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index def0ab85134a..e13fea25ecb8 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4560,6 +4560,7 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
if (version == 1) {
ignore_offset = false;
+ size = min_t(u32, loi->size, SZ_64K);
} else {
/* Only accept flags we have defined so far */
if (loi->flags & ~(BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET)) {
@@ -4567,6 +4568,7 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
goto out_loi;
}
ignore_offset = loi->flags &
BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET;
+ size = min_t(u32, loi->size, SZ_16M);
}
path = btrfs_alloc_path();
@@ -4575,7 +4577,6 @@ static long btrfs_ioctl_logical_to_ino(struct
btrfs_fs_info *fs_info,
goto out;
}
- size = min_t(u32, loi->size, SZ_64K);
inodes = init_data_container(size);
if (IS_ERR(inodes)) {
ret = PTR_ERR(inodes);
--
2.11.0
--
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
