Re: [PATCH] Btrfs: rework outstanding_extents

[Lu Fengqi <lufq.fnst@cn.fujitsu.com>]

On Mon, Sep 25, 2017 at 04:00:09PM +0200, David Sterba wrote:
>On Thu, Aug 24, 2017 at 05:31:15PM +0800, Lu Fengqi wrote:
>> On Thu, Aug 17, 2017 at 01:01:29PM -0400, josef@toxicpanda.com wrote:
>> >From: Josef Bacik <jbacik@fb.com>
>> >
>> >Right now we do a lot of weird hoops around outstanding_extents in order
>> >to keep the extent count consistent.  This is because we logically
>> >transfer the outstanding_extent count from the initial reservation
>> >through the set_delalloc_bits.  This makes it pretty difficult to get a
>> >handle on how and when we need to mess with outstanding_extents.
>> >
>> >Fix this by revamping the rules of how we deal with outstanding_extents.
>> >Now instead everybody that is holding on to a delalloc extent is
>> >required to increase the outstanding extents count for itself.  This
>> >means we'll have something like this
>> >
>> >btrfs_dealloc_reserve_metadata	- outstanding_extents = 1
>> > btrfs_set_delalloc		- outstanding_extents = 2
>> >btrfs_release_delalloc_extents	- outstanding_extents = 1
>> >
>> >for an initial file write.  Now take the append write where we extend an
>> >existing delalloc range but still under the maximum extent size
>> >
>> >btrfs_delalloc_reserve_metadata - outstanding_extents = 2
>> >  btrfs_set_delalloc
>> >    btrfs_set_bit_hook		- outstanding_extents = 3
>> >    btrfs_merge_bit_hook	- outstanding_extents = 2
>> >btrfs_release_delalloc_extents	- outstanding_extnets = 1
>> >
>> >In order to make the ordered extent transition we of course must now
>> >make ordered extents carry their own outstanding_extent reservation, so
>> >for cow_file_range we end up with
>> >
>> >btrfs_add_ordered_extent	- outstanding_extents = 2
>> >clear_extent_bit		- outstanding_extents = 1
>> >btrfs_remove_ordered_extent	- outstanding_extents = 0
>> >
>> >This makes all manipulations of outstanding_extents much more explicit.
>> >Every successful call to btrfs_reserve_delalloc_metadata _must_ now be
>> >combined with btrfs_release_delalloc_extents, even in the error case, as
>> >that is the only function that actually modifies the
>> >outstanding_extents counter.
>> 
>> s/btrfs_reserve_delalloc_metadata/btrfs_delalloc_reserve_metadata
>> s/btrfs_release_delalloc_extents/btrfs_delalloc_release_extents
>> 
>> Acked-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>
>BTW,
>
>Documentation/process/5.Posting.rst
>
>216  - Acked-by: indicates an agreement by another developer (often a
>217    maintainer of the relevant code) that the patch is appropriate for
>218    inclusion into the kernel.
>
>and is commonly used by maintainers when someone else modifies the
>respective codebase by tree-wide, API or similar.
>
>If you agree with the way the patch is solving a particular problem and
>think it's important enough to be stated, you can write that in plain
>text.
>
>

Sorry for any inconvenience caused. I misunderstood about the meaning of
Acked-by.

I just want to express that it looks good to me.

-- 
Thanks,
Lu