| Message ID | 20200303071409.57982-1-wqu@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: Move generic backref cache build functions to backref.c | expand |
On Tue, Mar 03, 2020 at 03:13:50PM +0800, Qu Wenruo wrote: > The patchset is based on previous backref_cache_refactor branch, which > is further based on misc-next. > > The whole series can be fetched from github: > https://github.com/adam900710/linux/tree/backref_cache_code_move > > All the patches in previous branch is not touched at all, thus they are > not re-sent in this patchset. The patches are cleanups and code moving, please fix the coding style issues you find. * missing lines between declarations and statements * exported functions need btrfs_ prefix * comments should start with an upper case letter unless it's an identifier, formatted to 80 columns As this patchset depends on another one I'm not sure if it's right time to update it now, before the other one is merged as I think the same code is touched and this would cause extra work. Overall it makes sensed to add more to backref.[hc] and export that as an internal API.
On 3/3/20 2:13 AM, Qu Wenruo wrote: > The patchset is based on previous backref_cache_refactor branch, which > is further based on misc-next. > > The whole series can be fetched from github: > https://github.com/adam900710/linux/tree/backref_cache_code_move > > All the patches in previous branch is not touched at all, thus they are > not re-sent in this patchset. > > > Currently there are 3 major parts of build_backref_tree(): > - ITERATION > This will do a breadth-first search, starts from the target bytenr, > and queue all parents into the backref cache. > The result is a temporary map, which is only single-directional, and > involved new backref nodes are not yet inserted into the cache. > > - WEAVING > Finish the map to make it bi-directional, and insert new nodes into > the cache. > > - CLEANUP > Cleanup the useless nodes, either remove it completely or add them > into the cache as detached. > I've found a bunch of bugs in the backref code while fixing Zygo's problem, you are probably going to want to wait for my patches to go in before you start moving things around, because it's going to conflict a bunch. Thanks, Josef
On 2020/3/4 上午5:22, Josef Bacik wrote: > On 3/3/20 2:13 AM, Qu Wenruo wrote: >> The patchset is based on previous backref_cache_refactor branch, which >> is further based on misc-next. >> >> The whole series can be fetched from github: >> https://github.com/adam900710/linux/tree/backref_cache_code_move >> >> All the patches in previous branch is not touched at all, thus they are >> not re-sent in this patchset. >> >> >> Currently there are 3 major parts of build_backref_tree(): >> - ITERATION >> This will do a breadth-first search, starts from the target bytenr, >> and queue all parents into the backref cache. >> The result is a temporary map, which is only single-directional, and >> involved new backref nodes are not yet inserted into the cache. >> >> - WEAVING >> Finish the map to make it bi-directional, and insert new nodes into >> the cache. >> >> - CLEANUP >> Cleanup the useless nodes, either remove it completely or add them >> into the cache as detached. >> > > I've found a bunch of bugs in the backref code while fixing Zygo's > problem, you are probably going to want to wait for my patches to go in > before you start moving things around, because it's going to conflict a > bunch. Thanks, No problem, it's expected to have some comments even for previous patchset. So rebasing is expected. Thanks, Qu > > Josef
On Wed, Mar 04, 2020 at 12:54:24PM +0800, Qu Wenruo wrote: > > > On 2020/3/4 上午5:22, Josef Bacik wrote: > > On 3/3/20 2:13 AM, Qu Wenruo wrote: > >> The patchset is based on previous backref_cache_refactor branch, which > >> is further based on misc-next. > >> > >> The whole series can be fetched from github: > >> https://github.com/adam900710/linux/tree/backref_cache_code_move > >> > >> All the patches in previous branch is not touched at all, thus they are > >> not re-sent in this patchset. > >> > >> > >> Currently there are 3 major parts of build_backref_tree(): > >> - ITERATION > >> This will do a breadth-first search, starts from the target bytenr, > >> and queue all parents into the backref cache. > >> The result is a temporary map, which is only single-directional, and > >> involved new backref nodes are not yet inserted into the cache. > >> > >> - WEAVING > >> Finish the map to make it bi-directional, and insert new nodes into > >> the cache. > >> > >> - CLEANUP > >> Cleanup the useless nodes, either remove it completely or add them > >> into the cache as detached. > >> > > > > I've found a bunch of bugs in the backref code while fixing Zygo's > > problem, you are probably going to want to wait for my patches to go in > > before you start moving things around, because it's going to conflict a > > bunch. Thanks, > > No problem, it's expected to have some comments even for previous patchset. > > So rebasing is expected. The fixes get merged first so they can be applied to older stable trees, the cleanups that move code are best done as the last thing in the patch queue. So this depends on timing and the amount of conflicts but we have like 2 rcs to go.
On 2020/3/4 上午12:30, David Sterba wrote: > On Tue, Mar 03, 2020 at 03:13:50PM +0800, Qu Wenruo wrote: >> The patchset is based on previous backref_cache_refactor branch, which >> is further based on misc-next. >> >> The whole series can be fetched from github: >> https://github.com/adam900710/linux/tree/backref_cache_code_move >> >> All the patches in previous branch is not touched at all, thus they are >> not re-sent in this patchset. > > The patches are cleanups and code moving, please fix the coding style > issues you find. > > * missing lines between declarations and statements > * exported functions need btrfs_ prefix I have some question about this. Sometimes the "btrfs_" prefix requirement looks too mechanical. Some functions, like backref_cache_init() is very obvious related to backref cache. I can't see any special benefit from adding a "btrfs_" prefix. Thanks, Qu > * comments should start with an upper case letter unless it's an > identifier, formatted to 80 columns > > As this patchset depends on another one I'm not sure if it's right time > to update it now, before the other one is merged as I think the same > code is touched and this would cause extra work. > > Overall it makes sensed to add more to backref.[hc] and export that as > an internal API. >
The patchset is based on previous backref_cache_refactor branch, which is further based on misc-next. The whole series can be fetched from github: https://github.com/adam900710/linux/tree/backref_cache_code_move All the patches in previous branch is not touched at all, thus they are not re-sent in this patchset. Currently there are 3 major parts of build_backref_tree(): - ITERATION This will do a breadth-first search, starts from the target bytenr, and queue all parents into the backref cache. The result is a temporary map, which is only single-directional, and involved new backref nodes are not yet inserted into the cache. - WEAVING Finish the map to make it bi-directional, and insert new nodes into the cache. - CLEANUP Cleanup the useless nodes, either remove it completely or add them into the cache as detached. For the ITERATION part, there are only limited locations coupled with relocation. And WEAVING part is completely independent from relocation. While the CLEANUP part, although it has some relocation related code, it's not a big hunk of code after all. So, this patchset will move the ITERATION part, extracted as backref_cache_add_one_tree_block(), and the WEAVING part, extracted as backref_cache_finish_upper_links(), to backref.c, as the basis for later reuse of backref_cache. Qu Wenruo (19): btrfs: Move backref node/edge/cache structure to backref.h btrfs: Rename tree_entry to simple_node and export it btrfs: relocation: Make reloc root search specific for relocation backref cache btrfs: relocation: Add backref_cache::pending_edge and backref_cache::useless_node members btrfs: relocation: Add backref_cache::fs_info member btrfs: Move backref_cache_init() to backref.c btrfs: Move alloc_backref_node() to backref.c btrfs: Move alloc_backref_edge() to backref.c btrfs: Move link_backref_edge() to backref.c btrfs: Move free_backref_node() and free_backref_edge() to backref.h btrfs: Move drop_backref_node() and needed facilities to backref.h btrfs: Rename remove_backref_node() to cleanup_backref_node() and move it to backref.c btrfs: Move backref_cache_cleanup() to backref.c btrfs: Rename backref_tree_panic() to backref_cache_panic(), and move it to backref.c btrfs: Rename should_ignore_root() to should_ignore_reloc_root() and export it btrfs: relocation: Open-code read_fs_root() for handle_one_tree_backref() btrfs: Rename handle_one_tree_block() to backref_cache_add_one_tree_block() and move it to backref.c btrfs: relocation: Move the target backref node insert code into finish_upper_links() btrfs: Rename finish_upper_links() to backref_cache_finish_upper_links() and move it to backref.c fs/btrfs/backref.c | 534 ++++++++++++++++++++++++++ fs/btrfs/backref.h | 226 +++++++++++ fs/btrfs/ctree.h | 3 + fs/btrfs/misc.h | 54 +++ fs/btrfs/relocation.c | 870 ++++-------------------------------------- 5 files changed, 892 insertions(+), 795 deletions(-)