| Message ID | 1422862299-7361-1-git-send-email-forrestl@synology.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi, Found a problem compile testing this. hole_size = key_offset - search_start; Should not that be key.offset ? TIA Ed Tomlinson On Monday, February 2, 2015 2:31:39 AM EST, Forrest Liu wrote: > If device tree has hole, find_free_dev_extent() cannot find available > address properly. > > The example below, has one BIG hole in device tree, and can only > allocate just one chunk in a transaction. > > item 9 key (1 DEV_EXTENT 273841913856) itemoff 15811 itemsize 48 > dev extent chunk_tree 3 > chunk objectid 256 chunk offset 272759783424 length 1073741824 > item 10 key (1 DEV_EXTENT 1071632089088) itemoff 15763 itemsize 48 > dev extent chunk_tree 3 > chunk objectid 256 chunk offset 1070549958656 length 1073741824 > item 11 key (1 DEV_EXTENT 1072705830912) itemoff 15715 itemsize 48 > dev extent chunk_tree 3 > chunk objectid 256 chunk offset 1071623700480 length > > Signed-off-by: Forrest Liu <forrestl@synology.com> > --- > fs/btrfs/volumes.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index da7e0e1..61be789 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1060,6 +1060,7 @@ static int contains_pending_extent(struct > btrfs_trans_handle *trans, > struct extent_map *em; > struct list_head *search_list = &trans->transaction->pending_chunks; > int ret = 0; > + u64 physical_start = *start; > > again: > list_for_each_entry(em, search_list, list) { > @@ -1070,9 +1071,9 @@ again: > for (i = 0; i < map->num_stripes; i++) { > if (map->stripes[i].dev != device) > continue; > - if (map->stripes[i].physical >= *start + len || > + if (map->stripes[i].physical >= physical_start + len || > map->stripes[i].physical + em->orig_block_len <= > - *start) > + physical_start) > continue; > *start = map->stripes[i].physical + > em->orig_block_len; > @@ -1195,8 +1196,14 @@ again: > */ > if (contains_pending_extent(trans, device, > &search_start, > - hole_size)) > - hole_size = 0; > + hole_size)) { > + if (key.offset >= search_start) > + hole_size = key_offset - search_start; > + else { > + WARN_ON(1); > + hole_size = 0; > + } > + } > > if (hole_size > max_hole_size) { > max_hole_start = search_start; -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, February 2, 2015 9:39:06 AM EST, Ed Tomlinson wrote: Hi Booting a kernel with the three patches: [PATCH] Btrfs: fix find_free_dev_extent() malfunction in case device tree has hole [PATCH] Btrfs: btrfs_release_extent_buffer_page() didn't free pages of dummy extent [PATCH] Btrfs: fix BUG_ON in btrfs_orphan_add() when delete unused block group generates lots of opps here (I hate to post an anemic report but my serial console was not recording so I do not have the opps). They occured when starting X and, If I read them correctly, had something to do with extents. Anyone else? Thanks Ed Tomlinson > Hi, > > Found a problem compile testing this. > > hole_size = key_offset - search_start; > > Should not that be key.offset ? > > TIA > Ed Tomlinson > > > On Monday, February 2, 2015 2:31:39 AM EST, Forrest Liu wrote: >> If device tree has hole, find_free_dev_extent() cannot find available >> address properly. >> >> The example below, has one BIG hole in device tree, and can only >> allocate just one chunk in a transaction. >> >> item 9 key (1 DEV_EXTENT 273841913856) itemoff 15811 itemsize 48 ... > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-02-03 2:40 GMT+08:00 Ed Tomlinson <edt@aei.ca>: > On Monday, February 2, 2015 9:39:06 AM EST, Ed Tomlinson wrote: > > Hi > > Booting a kernel with the three patches: > [PATCH] Btrfs: fix find_free_dev_extent() malfunction in case device tree > has hole > [PATCH] Btrfs: btrfs_release_extent_buffer_page() didn't free pages of dummy > extent > [PATCH] Btrfs: fix BUG_ON in btrfs_orphan_add() when delete unused block > group > > generates lots of opps here (I hate to post an anemic report but my serial > console was not recording so I do not have the opps). They occured when > starting X and, If I read them correctly, had something to do with extents. > My fault, i should test these patches before i submit these patches. The oops was caused by patch "Btrfs: btrfs_release_extent_buffer_page() didn't free pages of dummy extent" I will resend these patches after test on linux-3.19-rc7. Thanks Forrest > Anyone else? > > Thanks > Ed Tomlinson > >> Hi, >> >> Found a problem compile testing this. >> hole_size = key_offset - search_start; >> >> Should not that be key.offset ? >> >> TIA >> Ed Tomlinson >> >> >> On Monday, February 2, 2015 2:31:39 AM EST, Forrest Liu wrote: >>> >>> If device tree has hole, find_free_dev_extent() cannot find available >>> address properly. >>> >>> The example below, has one BIG hole in device tree, and can only >>> allocate just one chunk in a transaction. >>> >>> item 9 key (1 DEV_EXTENT 273841913856) itemoff 15811 itemsize 48 ... >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Its a great idea to test the patches before submitting. However I think its even more importent to tell us the state of testing. eg. this is an RFC or in production in abc's kernel, and this version is untested or has been compile tested, boot tested, improves xfstests (before x failures out of z, after the patch the number of failures was y, where y<x). A little bit of info like this will help those of us using the info in the lists and should aid getting your patches into the kernel faster. Thanks Ed On Tuesday, February 3, 2015 6:36:40 AM EST, Forrest Liu wrote: > 2015-02-03 2:40 GMT+08:00 Ed Tomlinson <edt@aei.ca>: >> On Monday, February 2, 2015 9:39:06 AM EST, Ed Tomlinson wrote: >> >> Hi >> >> Booting a kernel with the three patches: >> [PATCH] Btrfs: fix find_free_dev_extent() malfunction in case device tree >> has hole ... > > My fault, i should test these patches before i submit these patches. > The oops was caused by patch > "Btrfs: btrfs_release_extent_buffer_page() didn't free pages of > dummy extent" > > I will resend these patches after test on linux-3.19-rc7. > > Thanks > Forrest > >> Anyone else? >> >> Thanks >> Ed Tomlinson >> ... > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 02, 2015 at 03:31:39PM +0800, Forrest Liu wrote: > If device tree has hole, find_free_dev_extent() cannot find available > address properly. > > The example below, has one BIG hole in device tree, and can only > allocate just one chunk in a transaction. > > item 9 key (1 DEV_EXTENT 273841913856) itemoff 15811 itemsize 48 > dev extent chunk_tree 3 > chunk objectid 256 chunk offset 272759783424 length 1073741824 > item 10 key (1 DEV_EXTENT 1071632089088) itemoff 15763 itemsize 48 > dev extent chunk_tree 3 > chunk objectid 256 chunk offset 1070549958656 length 1073741824 > item 11 key (1 DEV_EXTENT 1072705830912) itemoff 15715 itemsize 48 > dev extent chunk_tree 3 > chunk objectid 256 chunk offset 1071623700480 length > > Signed-off-by: Forrest Liu <forrestl@synology.com> > --- > fs/btrfs/volumes.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index da7e0e1..61be789 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1060,6 +1060,7 @@ static int contains_pending_extent(struct btrfs_trans_handle *trans, > struct extent_map *em; > struct list_head *search_list = &trans->transaction->pending_chunks; > int ret = 0; > + u64 physical_start = *start; > > again: > list_for_each_entry(em, search_list, list) { > @@ -1070,9 +1071,9 @@ again: > for (i = 0; i < map->num_stripes; i++) { > if (map->stripes[i].dev != device) > continue; > - if (map->stripes[i].physical >= *start + len || > + if (map->stripes[i].physical >= physical_start + len || > map->stripes[i].physical + em->orig_block_len <= > - *start) > + physical_start) > continue; > *start = map->stripes[i].physical + > em->orig_block_len; This part looks good. > @@ -1195,8 +1196,14 @@ again: > */ > if (contains_pending_extent(trans, device, > &search_start, > - hole_size)) > - hole_size = 0; > + hole_size)) { > + if (key.offset >= search_start) > + hole_size = key_offset - search_start; > + else { > + WARN_ON(1); > + hole_size = 0; I'd prefer WARN_ON_ONCE(1) since key.offset should not be less than search_start by design. Other looks good. Reviewed-by: Liu Bo <bo.li.liu@oracle.com> Thanks, -liubo > + } > + } > > if (hole_size > max_hole_size) { > max_hole_start = search_start; > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index da7e0e1..61be789 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1060,6 +1060,7 @@ static int contains_pending_extent(struct btrfs_trans_handle *trans, struct extent_map *em; struct list_head *search_list = &trans->transaction->pending_chunks; int ret = 0; + u64 physical_start = *start; again: list_for_each_entry(em, search_list, list) { @@ -1070,9 +1071,9 @@ again: for (i = 0; i < map->num_stripes; i++) { if (map->stripes[i].dev != device) continue; - if (map->stripes[i].physical >= *start + len || + if (map->stripes[i].physical >= physical_start + len || map->stripes[i].physical + em->orig_block_len <= - *start) + physical_start) continue; *start = map->stripes[i].physical + em->orig_block_len; @@ -1195,8 +1196,14 @@ again: */ if (contains_pending_extent(trans, device, &search_start, - hole_size)) - hole_size = 0; + hole_size)) { + if (key.offset >= search_start) + hole_size = key_offset - search_start; + else { + WARN_ON(1); + hole_size = 0; + } + } if (hole_size > max_hole_size) { max_hole_start = search_start;
If device tree has hole, find_free_dev_extent() cannot find available address properly. The example below, has one BIG hole in device tree, and can only allocate just one chunk in a transaction. item 9 key (1 DEV_EXTENT 273841913856) itemoff 15811 itemsize 48 dev extent chunk_tree 3 chunk objectid 256 chunk offset 272759783424 length 1073741824 item 10 key (1 DEV_EXTENT 1071632089088) itemoff 15763 itemsize 48 dev extent chunk_tree 3 chunk objectid 256 chunk offset 1070549958656 length 1073741824 item 11 key (1 DEV_EXTENT 1072705830912) itemoff 15715 itemsize 48 dev extent chunk_tree 3 chunk objectid 256 chunk offset 1071623700480 length Signed-off-by: Forrest Liu <forrestl@synology.com> --- fs/btrfs/volumes.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)