Re: [PATCH 00/15] Add delayed-refs support to btrfs-progs

2018-09-12 Thread David Sterba
On Wed, Sep 12, 2018 at 07:51:39PM +0800, Su Yue wrote:
> Actually, now kdave/devel still fails at fsck-tests/020 due to
> version 1st of this patchset. See the thread please
> https://www.spinics.net/lists/linux-btrfs/msg81675.html
> 
> Nikolay's V2 patchset should slove the problem.
> You may have known the situation, this mail is just a gentle reminder :).

I'll replace the patches today, thanks.


Re: [PATCH 00/15] Add delayed-refs support to btrfs-progs

2018-09-12 Thread Su Yue




On 2018/7/16 11:39 PM, David Sterba wrote:

On Fri, Jun 08, 2018 at 03:47:43PM +0300, Nikolay Borisov wrote:

Hello,
 
Here is a series which adds support for delayed refs. This is needed to enable

later work on adding freespace tree repair code. Additionally, it results in
more code sharing between kernel/user space.

Patches 1-9 are simple prep patches removing some arguments, causing problems
later. They can go independently of the delayed refs work. They don't introduce
any functional changes. Next, patches 10-13 introduce the needed infrastructure
to for delayed refs without actually activating it. Patch 14 finally wires it
up by adding the necessary call outs to btrfs_run_delayed refs and reworking the
extent addition/freeing functions. With all of this done, patch 15 finally
removes the old code.

This series passes all btrfs progs fsck and misc tests + fuzz tests apart from
fuzz-003/007/009 - but those fail without this series so it's unlikely it's
caused by it.

Nikolay Borisov (15):
   btrfs-progs: Remove root argument from pin_down_bytes
   btrfs-progs: Remove root argument from btrfs_del_csums
   btrfs-progs: Add functions to modify the used space by a root
   btrfs-progs: Refactor the root used bytes are updated
   btrfs-progs: Make update_block_group take fs_info instead of root
   btrfs-progs: check: Drop trans/root arguments from free_extent_hook
   btrfs-progs: Remove root argument from __free_extent
   btrfs-progs: Remove root argument from alloc_reserved_tree_block
   btrfs-progs: Always pass 0 for offset when calling btrfs_free_extent
 for btree blocks.
   btrfs-progs: Add boolean to signal whether we are re-initing extent
 tree
   btrfs-progs: Add delayed refs infrastructure
   btrfs-progs: Add __free_extent2 function
   btrfs-progs: Add alloc_reserved_tree_block2 function
   btrfs-progs: Wire up delayed refs
   btrfs-progs: Remove old delayed refs infrastructure


Added to devel. There were some patch-to-patch compilation issues,
function alloc_reserved_tree_block2 used earlier than defined so I
reordered the patches to fix that.

The CI fails at test check/020-extent-ref-cases but it works on my
machine so it's not caused by the patchset.


Hi, David
Actually, now kdave/devel still fails at fsck-tests/020 due to
version 1st of this patchset. See the thread please
https://www.spinics.net/lists/linux-btrfs/msg81675.html

Nikolay's V2 patchset should slove the problem.
You may have known the situation, this mail is just a gentle reminder :).

Thanks,
Su

--
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 00/15] Add delayed-refs support to btrfs-progs

2018-07-16 Thread David Sterba
On Fri, Jun 08, 2018 at 03:47:43PM +0300, Nikolay Borisov wrote:
> Hello,
>   
>   
>   
> Here is a series which adds support for delayed refs. This is needed to 
> enable  
> later work on adding freespace tree repair code. Additionally, it results in  
> more code sharing between kernel/user space.
> 
> Patches 1-9 are simple prep patches removing some arguments, causing problems
> later. They can go independently of the delayed refs work. They don't 
> introduce
> any functional changes. Next, patches 10-13 introduce the needed 
> infrastructure
> to for delayed refs without actually activating it. Patch 14 finally wires it
> up by adding the necessary call outs to btrfs_run_delayed refs and reworking 
> the
> extent addition/freeing functions. With all of this done, patch 15 finally
> removes the old code.
> 
> This series passes all btrfs progs fsck and misc tests + fuzz tests apart from
> fuzz-003/007/009 - but those fail without this series so it's unlikely it's
> caused by it.
> 
> Nikolay Borisov (15):
>   btrfs-progs: Remove root argument from pin_down_bytes
>   btrfs-progs: Remove root argument from btrfs_del_csums
>   btrfs-progs: Add functions to modify the used space by a root
>   btrfs-progs: Refactor the root used bytes are updated
>   btrfs-progs: Make update_block_group take fs_info instead of root
>   btrfs-progs: check: Drop trans/root arguments from free_extent_hook
>   btrfs-progs: Remove root argument from __free_extent
>   btrfs-progs: Remove root argument from alloc_reserved_tree_block
>   btrfs-progs: Always pass 0 for offset when calling btrfs_free_extent
> for btree blocks.
>   btrfs-progs: Add boolean to signal whether we are re-initing extent
> tree
>   btrfs-progs: Add delayed refs infrastructure
>   btrfs-progs: Add __free_extent2 function
>   btrfs-progs: Add alloc_reserved_tree_block2 function
>   btrfs-progs: Wire up delayed refs
>   btrfs-progs: Remove old delayed refs infrastructure

Added to devel. There were some patch-to-patch compilation issues,
function alloc_reserved_tree_block2 used earlier than defined so I
reordered the patches to fix that.

The CI fails at test check/020-extent-ref-cases but it works on my
machine so it's not caused by the patchset.
--
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 00/15] Add delayed-refs support to btrfs-progs

2018-06-08 Thread Qu Wenruo



On 2018年06月08日 22:08, Nikolay Borisov wrote:
> 
> 
> On  8.06.2018 16:50, Qu Wenruo wrote:
>>  details?
>> Personally speaking, I'd like to avoid introducing complex delayed-ref
>> into btrfs-progs if possible.
>>
>> And in my (possibly wrong) understanding, the main purpose of
>> delayed-ref is to reduce the race on extent tree, thus to improve
>> performance.
>> However in btrfs-progs, it's the least important aspect.
>>
>> So extra comment on this is appreciated.
> 
> So in order to have freespace tree repair code working I needed to hook
> up its add_to_free_space_tree/remove_from_free_space_tree to
> alloc_reserved_tree_block/__free_extent. In my testing this lead to a
> very deep recursion - it crashed on 58k call frames. So the idea was to
> have delayed refs which would record and accumulate modifications and
> then the freespace tree freeing code would piggy back on them to rely on
> correct operation.

In fact, I have a pretty nasty idea on this problem.
Mark one or more metadata chunks without free space tree cache.

Then at least recursion could be easily resolved (although need extra
extent allocation hook to handle fst allocation)

> 
> I guess I could try and debug the freespace code and see why I was going
> into this infinite recursion so to speak.
> 
> Also the delayed refs code in progs is actually a lot simpler than the
> kernel counterpart due to the lack of locking.

Right.
And no need to do the async delayed ref execution should also makes
things easier.

> One more benefit of
> having this code in progs is the fact one can go through it with a
> debugger and really inspect/understand how it works

Indeed, this makes a lot of sense.

I'll take some time to do more review on this patchset, and dig deeper
into delayed-ref facility.

Thanks,
Qu

> - i.e addition of
> refs, selection of refs etc. Furthermore, it at least unifies the logic
> between kernel and userspace, since right now there is code which mimics
> the delayed refs - check the code being removed in the last patch.
> --
> 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
> 
--
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 00/15] Add delayed-refs support to btrfs-progs

2018-06-08 Thread Nikolay Borisov



On  8.06.2018 16:50, Qu Wenruo wrote:
>  details?
> Personally speaking, I'd like to avoid introducing complex delayed-ref
> into btrfs-progs if possible.
> 
> And in my (possibly wrong) understanding, the main purpose of
> delayed-ref is to reduce the race on extent tree, thus to improve
> performance.
> However in btrfs-progs, it's the least important aspect.
> 
> So extra comment on this is appreciated.

So in order to have freespace tree repair code working I needed to hook
up its add_to_free_space_tree/remove_from_free_space_tree to
alloc_reserved_tree_block/__free_extent. In my testing this lead to a
very deep recursion - it crashed on 58k call frames. So the idea was to
have delayed refs which would record and accumulate modifications and
then the freespace tree freeing code would piggy back on them to rely on
correct operation.

I guess I could try and debug the freespace code and see why I was going
into this infinite recursion so to speak.

Also the delayed refs code in progs is actually a lot simpler than the
kernel counterpart due to the lack of locking. One more benefit of
having this code in progs is the fact one can go through it with a
debugger and really inspect/understand how it works - i.e addition of
refs, selection of refs etc. Furthermore, it at least unifies the logic
between kernel and userspace, since right now there is code which mimics
the delayed refs - check the code being removed in the last patch.
--
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 00/15] Add delayed-refs support to btrfs-progs

2018-06-08 Thread Qu Wenruo



On 2018年06月08日 20:47, Nikolay Borisov wrote:
> Hello,
>   
>   
>   
> Here is a series which adds support for delayed refs. This is needed to 
> enable  
> later work on adding freespace tree repair code.

Would it be possible to explain this in details?
Personally speaking, I'd like to avoid introducing complex delayed-ref
into btrfs-progs if possible.

And in my (possibly wrong) understanding, the main purpose of
delayed-ref is to reduce the race on extent tree, thus to improve
performance.
However in btrfs-progs, it's the least important aspect.

So extra comment on this is appreciated.

Thanks,
Qu

> Additionally, it results in  
> more code sharing between kernel/user space.
> 
> Patches 1-9 are simple prep patches removing some arguments, causing problems
> later. They can go independently of the delayed refs work. They don't 
> introduce
> any functional changes. Next, patches 10-13 introduce the needed 
> infrastructure
> to for delayed refs without actually activating it. Patch 14 finally wires it
> up by adding the necessary call outs to btrfs_run_delayed refs and reworking 
> the
> extent addition/freeing functions. With all of this done, patch 15 finally
> removes the old code.
> 
> This series passes all btrfs progs fsck and misc tests + fuzz tests apart from
> fuzz-003/007/009 - but those fail without this series so it's unlikely it's
> caused by it.
> 
> Nikolay Borisov (15):
>   btrfs-progs: Remove root argument from pin_down_bytes
>   btrfs-progs: Remove root argument from btrfs_del_csums
>   btrfs-progs: Add functions to modify the used space by a root
>   btrfs-progs: Refactor the root used bytes are updated
>   btrfs-progs: Make update_block_group take fs_info instead of root
>   btrfs-progs: check: Drop trans/root arguments from free_extent_hook
>   btrfs-progs: Remove root argument from __free_extent
>   btrfs-progs: Remove root argument from alloc_reserved_tree_block
>   btrfs-progs: Always pass 0 for offset when calling btrfs_free_extent
> for btree blocks.
>   btrfs-progs: Add boolean to signal whether we are re-initing extent
> tree
>   btrfs-progs: Add delayed refs infrastructure
>   btrfs-progs: Add __free_extent2 function
>   btrfs-progs: Add alloc_reserved_tree_block2 function
>   btrfs-progs: Wire up delayed refs
>   btrfs-progs: Remove old delayed refs infrastructure
> 
>  Makefile  |   3 +-
>  btrfs-corrupt-block.c |   2 +-
>  check/main.c  |   8 +-
>  ctree.c   |  29 ++-
>  ctree.h   |  11 +-
>  delayed-ref.c | 608 
> ++
>  delayed-ref.h | 225 +++
>  extent-tree.c | 604 +
>  file-item.c   |  20 +-
>  kerncompat.h  |   8 +
>  transaction.c |  25 +++
>  transaction.h |   5 +
>  12 files changed, 1280 insertions(+), 268 deletions(-)
>  create mode 100644 delayed-ref.c
>  create mode 100644 delayed-ref.h
> 
--
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 00/15] Add delayed-refs support to btrfs-progs

2018-06-08 Thread Nikolay Borisov
Hello,  

Here is a series which adds support for delayed refs. This is needed to enable  
later work on adding freespace tree repair code. Additionally, it results in  
more code sharing between kernel/user space.

Patches 1-9 are simple prep patches removing some arguments, causing problems
later. They can go independently of the delayed refs work. They don't introduce
any functional changes. Next, patches 10-13 introduce the needed infrastructure
to for delayed refs without actually activating it. Patch 14 finally wires it
up by adding the necessary call outs to btrfs_run_delayed refs and reworking the
extent addition/freeing functions. With all of this done, patch 15 finally
removes the old code.

This series passes all btrfs progs fsck and misc tests + fuzz tests apart from
fuzz-003/007/009 - but those fail without this series so it's unlikely it's
caused by it.

Nikolay Borisov (15):
  btrfs-progs: Remove root argument from pin_down_bytes
  btrfs-progs: Remove root argument from btrfs_del_csums
  btrfs-progs: Add functions to modify the used space by a root
  btrfs-progs: Refactor the root used bytes are updated
  btrfs-progs: Make update_block_group take fs_info instead of root
  btrfs-progs: check: Drop trans/root arguments from free_extent_hook
  btrfs-progs: Remove root argument from __free_extent
  btrfs-progs: Remove root argument from alloc_reserved_tree_block
  btrfs-progs: Always pass 0 for offset when calling btrfs_free_extent
for btree blocks.
  btrfs-progs: Add boolean to signal whether we are re-initing extent
tree
  btrfs-progs: Add delayed refs infrastructure
  btrfs-progs: Add __free_extent2 function
  btrfs-progs: Add alloc_reserved_tree_block2 function
  btrfs-progs: Wire up delayed refs
  btrfs-progs: Remove old delayed refs infrastructure

 Makefile  |   3 +-
 btrfs-corrupt-block.c |   2 +-
 check/main.c  |   8 +-
 ctree.c   |  29 ++-
 ctree.h   |  11 +-
 delayed-ref.c | 608 ++
 delayed-ref.h | 225 +++
 extent-tree.c | 604 +
 file-item.c   |  20 +-
 kerncompat.h  |   8 +
 transaction.c |  25 +++
 transaction.h |   5 +
 12 files changed, 1280 insertions(+), 268 deletions(-)
 create mode 100644 delayed-ref.c
 create mode 100644 delayed-ref.h

-- 
2.7.4

--
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