lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALv5hoSEPXi3dNXg1DMKJQ3R3OnzVew+tNJQssWGt2g4O8aSiQ@mail.gmail.com>
Date: Wed, 30 Oct 2024 11:22:40 +0800
From: hs wang <iamhswang@...il.com>
To: Qu Wenruo <quwenruo.btrfs@....com>
Cc: linux-btrfs@...r.kernel.org, clm@...com, josef@...icpanda.com, 
	dsterba@...e.com, wqu@...e.com, boris@....io, linux-kernel@...r.kernel.org, 
	Haisu Wang <haisuwang@...cent.com>
Subject: Re: [PATCH 2/2] btrfs: simplify regions mark and keep start unchanged
 in err handling

Qu Wenruo <quwenruo.btrfs@....com> 于2024年10月30日周三 11:01写道:
>
>
>
> 在 2024/10/25 17:24, iamhswang@...il.com 写道:
> > From: Haisu Wang <haisuwang@...cent.com>
> >
> > Simplify the regions mark by using cur_alloc_size only to present
> > the reserved but may failed to alloced extent. Remove the ram_size
> > as well since it is always consistent to the cur_alloc_size in the
> > context. Advanced the start mark in normal path until extent succeed
> > alloced and keep the start unchanged in error handling path.
> >
> > PASSed the fstest generic/475 test for a hundred times with quota
> > enabled. And a modified generic/475 test by removing the sleep time
> > for a hundred times. About one tenth of the tests do enter the error
> > handling path due to fail to reserve extent.
> >
>
> Although this patch is already merged into for-next, it looks like the
> next patch will again change the error handling, mostly render the this
> one useless:
>
> https://lore.kernel.org/linux-btrfs/2a0925f0264daf90741ed0a7ba7ed4b4888cf778.1728725060.git.wqu@suse.com/
>
> The newer patch will change the error handling to a simpler one, so
> instead of 3 regions, there will be only 2.
>
Sounds better and a completely rewrite, i will catch up the details.

> There will be no change needed from your side, I will update my patches
> to solve the conflicts, just in case if you find the error handling is
> different in the future.
>

Thanks. Understanding the changes, may compile and play after the polishment.

Best regards,
Haisu Wang

> Thanks,
> Qu
>
> > Suggested-by: Qu Wenruo <wqu@...e.com>
> > Signed-off-by: Haisu Wang <haisuwang@...cent.com>
> > ---
> >   fs/btrfs/inode.c | 32 ++++++++++++++------------------
> >   1 file changed, 14 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 3646734a7e59..7e67a6d50be2 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -1359,7 +1359,6 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >       u64 alloc_hint = 0;
> >       u64 orig_start = start;
> >       u64 num_bytes;
> > -     unsigned long ram_size;
> >       u64 cur_alloc_size = 0;
> >       u64 min_alloc_size;
> >       u64 blocksize = fs_info->sectorsize;
> > @@ -1367,7 +1366,6 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >       struct extent_map *em;
> >       unsigned clear_bits;
> >       unsigned long page_ops;
> > -     bool extent_reserved = false;
> >       int ret = 0;
> >
> >       if (btrfs_is_free_space_inode(inode)) {
> > @@ -1421,8 +1419,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >               struct btrfs_ordered_extent *ordered;
> >               struct btrfs_file_extent file_extent;
> >
> > -             cur_alloc_size = num_bytes;
> > -             ret = btrfs_reserve_extent(root, cur_alloc_size, cur_alloc_size,
> > +             ret = btrfs_reserve_extent(root, num_bytes, num_bytes,
> >                                          min_alloc_size, 0, alloc_hint,
> >                                          &ins, 1, 1);
> >               if (ret == -EAGAIN) {
> > @@ -1453,9 +1450,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >               if (ret < 0)
> >                       goto out_unlock;
> >               cur_alloc_size = ins.offset;
> > -             extent_reserved = true;
> >
> > -             ram_size = ins.offset;
> >               file_extent.disk_bytenr = ins.objectid;
> >               file_extent.disk_num_bytes = ins.offset;
> >               file_extent.num_bytes = ins.offset;
> > @@ -1463,14 +1458,14 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >               file_extent.offset = 0;
> >               file_extent.compression = BTRFS_COMPRESS_NONE;
> >
> > -             lock_extent(&inode->io_tree, start, start + ram_size - 1,
> > +             lock_extent(&inode->io_tree, start, start + cur_alloc_size - 1,
> >                           &cached);
> >
> >               em = btrfs_create_io_em(inode, start, &file_extent,
> >                                       BTRFS_ORDERED_REGULAR);
> >               if (IS_ERR(em)) {
> >                       unlock_extent(&inode->io_tree, start,
> > -                                   start + ram_size - 1, &cached);
> > +                                   start + cur_alloc_size - 1, &cached);
> >                       ret = PTR_ERR(em);
> >                       goto out_reserve;
> >               }
> > @@ -1480,7 +1475,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >                                                    1 << BTRFS_ORDERED_REGULAR);
> >               if (IS_ERR(ordered)) {
> >                       unlock_extent(&inode->io_tree, start,
> > -                                   start + ram_size - 1, &cached);
> > +                                   start + cur_alloc_size - 1, &cached);
> >                       ret = PTR_ERR(ordered);
> >                       goto out_drop_extent_cache;
> >               }
> > @@ -1501,7 +1496,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >                        */
> >                       if (ret)
> >                               btrfs_drop_extent_map_range(inode, start,
> > -                                                         start + ram_size - 1,
> > +                                                         start + cur_alloc_size - 1,
> >                                                           false);
> >               }
> >               btrfs_put_ordered_extent(ordered);
> > @@ -1519,7 +1514,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >               page_ops = (keep_locked ? 0 : PAGE_UNLOCK);
> >               page_ops |= PAGE_SET_ORDERED;
> >
> > -             extent_clear_unlock_delalloc(inode, start, start + ram_size - 1,
> > +             extent_clear_unlock_delalloc(inode, start, start + cur_alloc_size - 1,
> >                                            locked_folio, &cached,
> >                                            EXTENT_LOCKED | EXTENT_DELALLOC,
> >                                            page_ops);
> > @@ -1529,7 +1524,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >                       num_bytes -= cur_alloc_size;
> >               alloc_hint = ins.objectid + ins.offset;
> >               start += cur_alloc_size;
> > -             extent_reserved = false;
> > +             cur_alloc_size = 0;
> >
> >               /*
> >                * btrfs_reloc_clone_csums() error, since start is increased
> > @@ -1545,7 +1540,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >       return ret;
> >
> >   out_drop_extent_cache:
> > -     btrfs_drop_extent_map_range(inode, start, start + ram_size - 1, false);
> > +     btrfs_drop_extent_map_range(inode, start, start + cur_alloc_size - 1, false);
> >   out_reserve:
> >       btrfs_dec_block_group_reservations(fs_info, ins.objectid);
> >       btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 1);
> > @@ -1599,13 +1594,12 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >        * to decrement again the data space_info's bytes_may_use counter,
> >        * therefore we do not pass it the flag EXTENT_CLEAR_DATA_RESV.
> >        */
> > -     if (extent_reserved) {
> > +     if (cur_alloc_size) {
> >               extent_clear_unlock_delalloc(inode, start,
> >                                            start + cur_alloc_size - 1,
> >                                            locked_folio, &cached, clear_bits,
> >                                            page_ops);
> >               btrfs_qgroup_free_data(inode, NULL, start, cur_alloc_size, NULL);
> > -             start += cur_alloc_size;
> >       }
> >
> >       /*
> > @@ -1614,11 +1608,13 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> >        * space_info's bytes_may_use counter, reserved in
> >        * btrfs_check_data_free_space().
> >        */
> > -     if (start < end) {
> > +     if (start + cur_alloc_size < end) {
> >               clear_bits |= EXTENT_CLEAR_DATA_RESV;
> > -             extent_clear_unlock_delalloc(inode, start, end, locked_folio,
> > +             extent_clear_unlock_delalloc(inode, start + cur_alloc_size,
> > +                                          end, locked_folio,
> >                                            &cached, clear_bits, page_ops);
> > -             btrfs_qgroup_free_data(inode, NULL, start, end - start + 1, NULL);
> > +             btrfs_qgroup_free_data(inode, NULL, start + cur_alloc_size,
> > +                             end - start - cur_alloc_size + 1, NULL);
> >       }
> >       return ret;
> >   }
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ