On 20/02/2019 16:11, Nikolay Borisov wrote:
> This commit changes the implementation of cow_file_range_async in order
> to get rid of the BUG_ON in the middle of the loop. Additionally it
> reworks the inner loop in the hopes of making it more understandable.
>
> Main change is that the number of chunks required to handle the given
> range is calculated before going into the loop and the logic of the loop
> just iterates the chunk count. Furthermore, the way memory is allocated
> is reworked and now the code does a single kmalloc with enough space to
> handle all chunks. Depending on whether compression is enabled or not
> chunks are either 1 (in non-compress case) or the range divided by 512k.
>
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> fs/btrfs/inode.c | 84 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 58 insertions(+), 26 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 05bbfd02ea49..546000779310 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -375,6 +375,7 @@ struct async_cow {
> unsigned int write_flags;
> struct list_head extents;
> struct btrfs_work work;
> + atomic_t *pending;
> };
>
> static noinline int add_async_extent(struct async_cow *cow,
> @@ -1181,7 +1182,12 @@ static noinline void async_cow_free(struct btrfs_work *work)
> async_cow = container_of(work, struct async_cow, work);
> if (async_cow->inode)
> btrfs_add_delayed_iput(async_cow->inode);
> - kfree(async_cow);
> + /*
> + * Since the pointer to 'pending' is at the beginning of the array of
> + * async_cow's, freeing it ensures the whole array has been freed.
> + */
> + if (atomic_dec_and_test(async_cow->pending))
> + kfree(async_cow->pending);
> }
[...]
> + /* Layout is [atomic_t][async_cow1][async_cowN].... */
> + async_cow = kmalloc(sizeof(atomic_t) + num_chunks*sizeof(*async_cow),
> + GFP_NOFS);
> + if (!async_cow) {
> + unsigned clear_bits = EXTENT_LOCKED | EXTENT_DELALLOC |
> + EXTENT_DELALLOC_NEW | EXTENT_DEFRAG |
> + EXTENT_DO_ACCOUNTING;
> + unsigned long page_ops = PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> + PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK |
> + PAGE_SET_ERROR;
> + extent_clear_unlock_delalloc(inode, start, end, 0, locked_page,
> + clear_bits, page_ops);
> + return -ENOMEM;
> + }
> +
> + p = (atomic_t *)async_cow;
> + async_cow = (struct async_cow *)((char *)async_cow + sizeof(atomic_t));
> + atomic_set(p, num_chunks);
Ugh! You're abusing an atomic_t * for something you want to actually
make this:
@@ -375,6 +375,7 @@ struct async_cow {
unsigned int write_flags;
struct list_head extents;
struct btrfs_work work;
+ atomic_t pending;
+ struct async_cow list[0];
};
and then have:
tmp = krealloc(async_cow, num_chunks * sizeof(*async_cow), GFP_NOFS);
if (!tmp) {
[...]
}
kfree(async_cow);
async_cow = tmp;
--
Johannes Thumshirn SUSE Labs Filesystems
jthumshirn@xxxxxxx +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
