* [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks @ 2019-08-28 2:33 Qu Wenruo 2019-08-28 2:33 ` [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid Qu Wenruo ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Qu Wenruo @ 2019-08-28 2:33 UTC (permalink / raw) To: linux-btrfs For SYSTEM chunks, despite the regular chunk item size limit, there is another limit due to system chunk array size. The extra limit is removed in a refactor, so just add it back. Fixes: e3ecdb3fdecf ("btrfs: factor out devs_max setting in __btrfs_alloc_chunk") Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/volumes.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index a447d3ec48d5..8b72d03738d9 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -4966,6 +4966,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, } else if (type & BTRFS_BLOCK_GROUP_SYSTEM) { max_stripe_size = SZ_32M; max_chunk_size = 2 * max_stripe_size; + devs_max = min_t(int, devs_max, BTRFS_MAX_DEVS_SYS_CHUNK); } else { btrfs_err(info, "invalid chunk type 0x%llx requested", type); -- 2.23.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid 2019-08-28 2:33 [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Qu Wenruo @ 2019-08-28 2:33 ` Qu Wenruo 2019-08-28 6:27 ` Nikolay Borisov 2019-09-10 9:07 ` Anand Jain 2019-08-28 6:35 ` [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Nikolay Borisov ` (2 subsequent siblings) 3 siblings, 2 replies; 10+ messages in thread From: Qu Wenruo @ 2019-08-28 2:33 UTC (permalink / raw) To: linux-btrfs; +Cc: Anand Jain [BUG] The following script will cause false alert on devid check. #!/bin/bash dev1=/dev/test/test dev2=/dev/test/scratch1 mnt=/mnt/btrfs umount $dev1 &> /dev/null umount $dev2 &> /dev/null umount $mnt &> /dev/null mkfs.btrfs -f $dev1 mount $dev1 $mnt _fail() { echo "!!! FAILED !!!" exit 1 } for ((i = 0; i < 4096; i++)); do btrfs dev add -f $dev2 $mnt || _fail btrfs dev del $dev1 $mnt || _fail dev_tmp=$dev1 dev1=$dev2 dev2=$dev_tmp done [CAUSE] Tree-checker uses BTRFS_MAX_DEVS() and BTRFS_MAX_DEVS_SYS_CHUNK() as upper limit for devid. But we can have devid holes just like above script. So the check for devid is incorrect and could cause false alert. [FIX] Just remove the whole devid check. We don't have any hard requirement for devid assignment. Furthermore, even devid get corrupted by bitflip, we still have dev extents verification at mount time, so corrupted data won't sneak into kernel. Reported-by: Anand Jain <anand.jain@oracle.com> Fixes: ab4ba2e13346 ("btrfs: tree-checker: Verify dev item") Signed-off-by: Qu Wenruo <wqu@suse.com> --- Changelog: v2: - Remove devid check completely As we already have verify_one_dev_extent(). v3: - Unexport BTRFS_MAX_DEVS() and BTRFS_MAX_DEVS_SYS_CHUNK macros - Update commit message to include the reproducer. --- fs/btrfs/tree-checker.c | 8 -------- fs/btrfs/volumes.c | 9 +++++++++ fs/btrfs/volumes.h | 9 --------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c index ccd5706199d7..15d1aa7cef1f 100644 --- a/fs/btrfs/tree-checker.c +++ b/fs/btrfs/tree-checker.c @@ -686,9 +686,7 @@ static void dev_item_err(const struct extent_buffer *eb, int slot, static int check_dev_item(struct extent_buffer *leaf, struct btrfs_key *key, int slot) { - struct btrfs_fs_info *fs_info = leaf->fs_info; struct btrfs_dev_item *ditem; - u64 max_devid = max(BTRFS_MAX_DEVS(fs_info), BTRFS_MAX_DEVS_SYS_CHUNK); if (key->objectid != BTRFS_DEV_ITEMS_OBJECTID) { dev_item_err(leaf, slot, @@ -696,12 +694,6 @@ static int check_dev_item(struct extent_buffer *leaf, key->objectid, BTRFS_DEV_ITEMS_OBJECTID); return -EUCLEAN; } - if (key->offset > max_devid) { - dev_item_err(leaf, slot, - "invalid devid: has=%llu expect=[0, %llu]", - key->offset, max_devid); - return -EUCLEAN; - } ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item); if (btrfs_device_id(leaf, ditem) != key->offset) { dev_item_err(leaf, slot, diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 8b72d03738d9..56f751192a6c 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -4901,6 +4901,15 @@ static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type) btrfs_set_fs_incompat(info, RAID56); } +#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \ + - sizeof(struct btrfs_chunk)) \ + / sizeof(struct btrfs_stripe) + 1) + +#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \ + - 2 * sizeof(struct btrfs_disk_key) \ + - 2 * sizeof(struct btrfs_chunk)) \ + / sizeof(struct btrfs_stripe) + 1) + static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, u64 start, u64 type) { diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 7f6aa1816409..789f983a98c5 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -273,15 +273,6 @@ struct btrfs_fs_devices { #define BTRFS_BIO_INLINE_CSUM_SIZE 64 -#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \ - - sizeof(struct btrfs_chunk)) \ - / sizeof(struct btrfs_stripe) + 1) - -#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \ - - 2 * sizeof(struct btrfs_disk_key) \ - - 2 * sizeof(struct btrfs_chunk)) \ - / sizeof(struct btrfs_stripe) + 1) - /* * we need the mirror number and stripe index to be passed around * the call chain while we are processing end_io (especially errors). -- 2.23.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid 2019-08-28 2:33 ` [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid Qu Wenruo @ 2019-08-28 6:27 ` Nikolay Borisov 2019-08-28 6:30 ` WenRuo Qu 2019-09-10 9:07 ` Anand Jain 1 sibling, 1 reply; 10+ messages in thread From: Nikolay Borisov @ 2019-08-28 6:27 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs; +Cc: Anand Jain On 28.08.19 г. 5:33 ч., Qu Wenruo wrote: > [BUG] > The following script will cause false alert on devid check. > #!/bin/bash > > dev1=/dev/test/test > dev2=/dev/test/scratch1 > mnt=/mnt/btrfs > > umount $dev1 &> /dev/null > umount $dev2 &> /dev/null > umount $mnt &> /dev/null > > mkfs.btrfs -f $dev1 > > mount $dev1 $mnt > > _fail() > { > echo "!!! FAILED !!!" > exit 1 > } > > for ((i = 0; i < 4096; i++)); do > btrfs dev add -f $dev2 $mnt || _fail > btrfs dev del $dev1 $mnt || _fail > dev_tmp=$dev1 > dev1=$dev2 > dev2=$dev_tmp > done Instead of putting the script here, can't it be turned into a fstest to ensure we don't regress in the future? > > [CAUSE] > Tree-checker uses BTRFS_MAX_DEVS() and BTRFS_MAX_DEVS_SYS_CHUNK() as > upper limit for devid. > But we can have devid holes just like above script. > > So the check for devid is incorrect and could cause false alert. > > [FIX] > Just remove the whole devid check. > We don't have any hard requirement for devid assignment. > > Furthermore, even devid get corrupted by bitflip, we still have dev > extents verification at mount time, so corrupted data won't sneak into > kernel. > > Reported-by: Anand Jain <anand.jain@oracle.com> > Fixes: ab4ba2e13346 ("btrfs: tree-checker: Verify dev item") > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > Changelog: > v2: > - Remove devid check completely > As we already have verify_one_dev_extent(). > v3: > - Unexport BTRFS_MAX_DEVS() and BTRFS_MAX_DEVS_SYS_CHUNK macros > - Update commit message to include the reproducer. > --- > fs/btrfs/tree-checker.c | 8 -------- > fs/btrfs/volumes.c | 9 +++++++++ > fs/btrfs/volumes.h | 9 --------- > 3 files changed, 9 insertions(+), 17 deletions(-) > > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c > index ccd5706199d7..15d1aa7cef1f 100644 > --- a/fs/btrfs/tree-checker.c > +++ b/fs/btrfs/tree-checker.c > @@ -686,9 +686,7 @@ static void dev_item_err(const struct extent_buffer *eb, int slot, > static int check_dev_item(struct extent_buffer *leaf, > struct btrfs_key *key, int slot) > { > - struct btrfs_fs_info *fs_info = leaf->fs_info; > struct btrfs_dev_item *ditem; > - u64 max_devid = max(BTRFS_MAX_DEVS(fs_info), BTRFS_MAX_DEVS_SYS_CHUNK); > > if (key->objectid != BTRFS_DEV_ITEMS_OBJECTID) { > dev_item_err(leaf, slot, > @@ -696,12 +694,6 @@ static int check_dev_item(struct extent_buffer *leaf, > key->objectid, BTRFS_DEV_ITEMS_OBJECTID); > return -EUCLEAN; > } > - if (key->offset > max_devid) { > - dev_item_err(leaf, slot, > - "invalid devid: has=%llu expect=[0, %llu]", > - key->offset, max_devid); > - return -EUCLEAN; > - } > ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item); > if (btrfs_device_id(leaf, ditem) != key->offset) { > dev_item_err(leaf, slot, > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 8b72d03738d9..56f751192a6c 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -4901,6 +4901,15 @@ static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type) > btrfs_set_fs_incompat(info, RAID56); > } > > +#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \ > + - sizeof(struct btrfs_chunk)) \ > + / sizeof(struct btrfs_stripe) + 1) > + > +#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \ > + - 2 * sizeof(struct btrfs_disk_key) \ > + - 2 * sizeof(struct btrfs_chunk)) \ > + / sizeof(struct btrfs_stripe) + 1) > + > static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > u64 start, u64 type) > { > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 7f6aa1816409..789f983a98c5 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -273,15 +273,6 @@ struct btrfs_fs_devices { > > #define BTRFS_BIO_INLINE_CSUM_SIZE 64 > > -#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \ > - - sizeof(struct btrfs_chunk)) \ > - / sizeof(struct btrfs_stripe) + 1) > - > -#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \ > - - 2 * sizeof(struct btrfs_disk_key) \ > - - 2 * sizeof(struct btrfs_chunk)) \ > - / sizeof(struct btrfs_stripe) + 1) > - > /* > * we need the mirror number and stripe index to be passed around > * the call chain while we are processing end_io (especially errors). > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid 2019-08-28 6:27 ` Nikolay Borisov @ 2019-08-28 6:30 ` WenRuo Qu 0 siblings, 0 replies; 10+ messages in thread From: WenRuo Qu @ 2019-08-28 6:30 UTC (permalink / raw) To: Nikolay Borisov, linux-btrfs; +Cc: Anand Jain On 2019/8/28 下午2:27, Nikolay Borisov wrote: > > > On 28.08.19 г. 5:33 ч., Qu Wenruo wrote: >> [BUG] >> The following script will cause false alert on devid check. >> #!/bin/bash >> >> dev1=/dev/test/test >> dev2=/dev/test/scratch1 >> mnt=/mnt/btrfs >> >> umount $dev1 &> /dev/null >> umount $dev2 &> /dev/null >> umount $mnt &> /dev/null >> >> mkfs.btrfs -f $dev1 >> >> mount $dev1 $mnt >> >> _fail() >> { >> echo "!!! FAILED !!!" >> exit 1 >> } >> >> for ((i = 0; i < 4096; i++)); do >> btrfs dev add -f $dev2 $mnt || _fail >> btrfs dev del $dev1 $mnt || _fail >> dev_tmp=$dev1 >> dev1=$dev2 >> dev2=$dev_tmp >> done > > Instead of putting the script here, can't it be turned into a fstest to > ensure we don't regress in the future? Sure, already crafting. Thanks, Qu > >> >> [CAUSE] >> Tree-checker uses BTRFS_MAX_DEVS() and BTRFS_MAX_DEVS_SYS_CHUNK() as >> upper limit for devid. >> But we can have devid holes just like above script. >> >> So the check for devid is incorrect and could cause false alert. >> >> [FIX] >> Just remove the whole devid check. >> We don't have any hard requirement for devid assignment. >> >> Furthermore, even devid get corrupted by bitflip, we still have dev >> extents verification at mount time, so corrupted data won't sneak into >> kernel. >> >> Reported-by: Anand Jain <anand.jain@oracle.com> >> Fixes: ab4ba2e13346 ("btrfs: tree-checker: Verify dev item") >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> Changelog: >> v2: >> - Remove devid check completely >> As we already have verify_one_dev_extent(). >> v3: >> - Unexport BTRFS_MAX_DEVS() and BTRFS_MAX_DEVS_SYS_CHUNK macros >> - Update commit message to include the reproducer. >> --- >> fs/btrfs/tree-checker.c | 8 -------- >> fs/btrfs/volumes.c | 9 +++++++++ >> fs/btrfs/volumes.h | 9 --------- >> 3 files changed, 9 insertions(+), 17 deletions(-) >> >> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c >> index ccd5706199d7..15d1aa7cef1f 100644 >> --- a/fs/btrfs/tree-checker.c >> +++ b/fs/btrfs/tree-checker.c >> @@ -686,9 +686,7 @@ static void dev_item_err(const struct extent_buffer *eb, int slot, >> static int check_dev_item(struct extent_buffer *leaf, >> struct btrfs_key *key, int slot) >> { >> - struct btrfs_fs_info *fs_info = leaf->fs_info; >> struct btrfs_dev_item *ditem; >> - u64 max_devid = max(BTRFS_MAX_DEVS(fs_info), BTRFS_MAX_DEVS_SYS_CHUNK); >> >> if (key->objectid != BTRFS_DEV_ITEMS_OBJECTID) { >> dev_item_err(leaf, slot, >> @@ -696,12 +694,6 @@ static int check_dev_item(struct extent_buffer *leaf, >> key->objectid, BTRFS_DEV_ITEMS_OBJECTID); >> return -EUCLEAN; >> } >> - if (key->offset > max_devid) { >> - dev_item_err(leaf, slot, >> - "invalid devid: has=%llu expect=[0, %llu]", >> - key->offset, max_devid); >> - return -EUCLEAN; >> - } >> ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item); >> if (btrfs_device_id(leaf, ditem) != key->offset) { >> dev_item_err(leaf, slot, >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 8b72d03738d9..56f751192a6c 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -4901,6 +4901,15 @@ static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type) >> btrfs_set_fs_incompat(info, RAID56); >> } >> >> +#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \ >> + - sizeof(struct btrfs_chunk)) \ >> + / sizeof(struct btrfs_stripe) + 1) >> + >> +#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \ >> + - 2 * sizeof(struct btrfs_disk_key) \ >> + - 2 * sizeof(struct btrfs_chunk)) \ >> + / sizeof(struct btrfs_stripe) + 1) >> + >> static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, >> u64 start, u64 type) >> { >> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h >> index 7f6aa1816409..789f983a98c5 100644 >> --- a/fs/btrfs/volumes.h >> +++ b/fs/btrfs/volumes.h >> @@ -273,15 +273,6 @@ struct btrfs_fs_devices { >> >> #define BTRFS_BIO_INLINE_CSUM_SIZE 64 >> >> -#define BTRFS_MAX_DEVS(info) ((BTRFS_MAX_ITEM_SIZE(info) \ >> - - sizeof(struct btrfs_chunk)) \ >> - / sizeof(struct btrfs_stripe) + 1) >> - >> -#define BTRFS_MAX_DEVS_SYS_CHUNK ((BTRFS_SYSTEM_CHUNK_ARRAY_SIZE \ >> - - 2 * sizeof(struct btrfs_disk_key) \ >> - - 2 * sizeof(struct btrfs_chunk)) \ >> - / sizeof(struct btrfs_stripe) + 1) >> - >> /* >> * we need the mirror number and stripe index to be passed around >> * the call chain while we are processing end_io (especially errors). >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid 2019-08-28 2:33 ` [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid Qu Wenruo 2019-08-28 6:27 ` Nikolay Borisov @ 2019-09-10 9:07 ` Anand Jain 2019-09-10 9:12 ` WenRuo Qu 1 sibling, 1 reply; 10+ messages in thread From: Anand Jain @ 2019-09-10 9:07 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c > index ccd5706199d7..15d1aa7cef1f 100644 > --- a/fs/btrfs/tree-checker.c > +++ b/fs/btrfs/tree-checker.c > @@ -686,9 +686,7 @@ static void dev_item_err(const struct extent_buffer *eb, int slot, > static int check_dev_item(struct extent_buffer *leaf, > struct btrfs_key *key, int slot) > { > - struct btrfs_fs_info *fs_info = leaf->fs_info; > struct btrfs_dev_item *ditem; > - u64 max_devid = max(BTRFS_MAX_DEVS(fs_info), BTRFS_MAX_DEVS_SYS_CHUNK); As I commented in v2. I see that BTRFS_MAX_DEVS_SYS_CHUNK is not being used anywhere else after this being removed. So good to delete the define. I am bit surprised as well if I am missing? Thanks, Anand ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid 2019-09-10 9:07 ` Anand Jain @ 2019-09-10 9:12 ` WenRuo Qu 2019-09-10 9:23 ` Anand Jain 0 siblings, 1 reply; 10+ messages in thread From: WenRuo Qu @ 2019-09-10 9:12 UTC (permalink / raw) To: Anand Jain, linux-btrfs On 2019/9/10 下午5:07, Anand Jain wrote: > > > > >> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c >> index ccd5706199d7..15d1aa7cef1f 100644 >> --- a/fs/btrfs/tree-checker.c >> +++ b/fs/btrfs/tree-checker.c >> @@ -686,9 +686,7 @@ static void dev_item_err(const struct >> extent_buffer *eb, int slot, >> static int check_dev_item(struct extent_buffer *leaf, >> struct btrfs_key *key, int slot) >> { >> - struct btrfs_fs_info *fs_info = leaf->fs_info; >> struct btrfs_dev_item *ditem; >> - u64 max_devid = max(BTRFS_MAX_DEVS(fs_info), >> BTRFS_MAX_DEVS_SYS_CHUNK); > > As I commented in v2. > I see that BTRFS_MAX_DEVS_SYS_CHUNK is not being used anywhere > else after this being removed. So good to delete the define. > I am bit surprised as well if I am missing? Please check the first patch. It adds back the reference to it as an early exit for btrfs_alloc_chunk(). Thanks, Qu > > Thanks, Anand > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid 2019-09-10 9:12 ` WenRuo Qu @ 2019-09-10 9:23 ` Anand Jain 0 siblings, 0 replies; 10+ messages in thread From: Anand Jain @ 2019-09-10 9:23 UTC (permalink / raw) To: WenRuo Qu, linux-btrfs On 10/9/19 5:12 PM, WenRuo Qu wrote: > > > On 2019/9/10 下午5:07, Anand Jain wrote: >> >> >> >> >>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c >>> index ccd5706199d7..15d1aa7cef1f 100644 >>> --- a/fs/btrfs/tree-checker.c >>> +++ b/fs/btrfs/tree-checker.c >>> @@ -686,9 +686,7 @@ static void dev_item_err(const struct >>> extent_buffer *eb, int slot, >>> static int check_dev_item(struct extent_buffer *leaf, >>> struct btrfs_key *key, int slot) >>> { >>> - struct btrfs_fs_info *fs_info = leaf->fs_info; >>> struct btrfs_dev_item *ditem; >>> - u64 max_devid = max(BTRFS_MAX_DEVS(fs_info), >>> BTRFS_MAX_DEVS_SYS_CHUNK); >> >> As I commented in v2. >> I see that BTRFS_MAX_DEVS_SYS_CHUNK is not being used anywhere >> else after this being removed. So good to delete the define. >> I am bit surprised as well if I am missing? > > Please check the first patch. > > It adds back the reference to it as an early exit for btrfs_alloc_chunk(). Oh. Right here in the same thread. Ok. Thanks. > Thanks, > Qu >> >> Thanks, Anand >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks 2019-08-28 2:33 [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Qu Wenruo 2019-08-28 2:33 ` [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid Qu Wenruo @ 2019-08-28 6:35 ` Nikolay Borisov 2019-09-10 14:01 ` Anand Jain 2019-10-24 19:40 ` David Sterba 3 siblings, 0 replies; 10+ messages in thread From: Nikolay Borisov @ 2019-08-28 6:35 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs On 28.08.19 г. 5:33 ч., Qu Wenruo wrote: > For SYSTEM chunks, despite the regular chunk item size limit, there is > another limit due to system chunk array size. > > The extra limit is removed in a refactor, so just add it back. > > Fixes: e3ecdb3fdecf ("btrfs: factor out devs_max setting in __btrfs_alloc_chunk") > Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/volumes.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index a447d3ec48d5..8b72d03738d9 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -4966,6 +4966,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > } else if (type & BTRFS_BLOCK_GROUP_SYSTEM) { > max_stripe_size = SZ_32M; > max_chunk_size = 2 * max_stripe_size; > + devs_max = min_t(int, devs_max, BTRFS_MAX_DEVS_SYS_CHUNK); > } else { > btrfs_err(info, "invalid chunk type 0x%llx requested", > type); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks 2019-08-28 2:33 [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Qu Wenruo 2019-08-28 2:33 ` [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid Qu Wenruo 2019-08-28 6:35 ` [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Nikolay Borisov @ 2019-09-10 14:01 ` Anand Jain 2019-10-24 19:40 ` David Sterba 3 siblings, 0 replies; 10+ messages in thread From: Anand Jain @ 2019-09-10 14:01 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs On 28/8/19 10:33 AM, Qu Wenruo wrote: > For SYSTEM chunks, despite the regular chunk item size limit, there is > another limit due to system chunk array size. > > The extra limit is removed in a refactor, so just add it back. > > Fixes: e3ecdb3fdecf ("btrfs: factor out devs_max setting in __btrfs_alloc_chunk") > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/volumes.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index a447d3ec48d5..8b72d03738d9 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -4966,6 +4966,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > } else if (type & BTRFS_BLOCK_GROUP_SYSTEM) { > max_stripe_size = SZ_32M; > max_chunk_size = 2 * max_stripe_size; > + devs_max = min_t(int, devs_max, BTRFS_MAX_DEVS_SYS_CHUNK); Reviewed-by: Anand Jain <anand.jain@oracle.com> Thanks, Anand > } else { > btrfs_err(info, "invalid chunk type 0x%llx requested", > type); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks 2019-08-28 2:33 [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Qu Wenruo ` (2 preceding siblings ...) 2019-09-10 14:01 ` Anand Jain @ 2019-10-24 19:40 ` David Sterba 3 siblings, 0 replies; 10+ messages in thread From: David Sterba @ 2019-10-24 19:40 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs On Wed, Aug 28, 2019 at 10:33:12AM +0800, Qu Wenruo wrote: > For SYSTEM chunks, despite the regular chunk item size limit, there is > another limit due to system chunk array size. > > The extra limit is removed in a refactor, so just add it back. > > Fixes: e3ecdb3fdecf ("btrfs: factor out devs_max setting in __btrfs_alloc_chunk") > Signed-off-by: Qu Wenruo <wqu@suse.com> So these patches fix test btrfs/194 failure, and given the regression also worth pushing to 5.4-rc. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-24 19:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-28 2:33 [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Qu Wenruo 2019-08-28 2:33 ` [PATCH v3 2/2] btrfs: tree-checker: Fix wrong check on max devid Qu Wenruo 2019-08-28 6:27 ` Nikolay Borisov 2019-08-28 6:30 ` WenRuo Qu 2019-09-10 9:07 ` Anand Jain 2019-09-10 9:12 ` WenRuo Qu 2019-09-10 9:23 ` Anand Jain 2019-08-28 6:35 ` [PATCH 1/2] btrfs: Consider system chunk array size for new SYSTEM chunks Nikolay Borisov 2019-09-10 14:01 ` Anand Jain 2019-10-24 19:40 ` David Sterba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).