On Tue, Mar 26, 2019 at 8:10 AM Nikolay Borisov <nbori...@suse.com> wrote:
>
> [CC'ing Filipe as he should now better ]
>
> On 25.03.19 г. 20:44 ч., Darrick J. Wong wrote:
> > On Mon, Mar 25, 2019 at 02:31:20PM +0200, Nikolay Borisov wrote:
> >> Here is v3 of the fitrim patches. Change since v2 [0]:
> >>
> >>  * Replaced BUG_ON with WARN_ON in patch 2
> >>
> >>  * Added RB to patches 04/05/06/09
> >>
> >>  * Squashed "btrfs: Transpose btrfs_close_devices/btrfs_mapping_tree_free 
> >> in close_ctree"
> >>  into patch 07. It was only sent to the mailing list as a followup.
> >>
> >>  * Rebased all patches on latest misc-next.
> >>
> >>  This has undergone multiple xfstest runs and I think is ready to be 
> >> merged.
> >>
> >> [0] 
> >> https://lore.kernel.org/linux-btrfs/20190211083510.27591-1-nbori...@suse.com/
> >>
> >>
> >> Jeff Mahoney (1):
> >>   btrfs: replace pending/pinned chunks lists with io tree
> >>
> >> Nikolay Borisov (11):
> >>   btrfs: Honour FITRIM range constraints during free space trim
> >
> > This is vaguely off-topic, but I noticed that you can FITRIM a btrfs
> > filesystem mounted nologreplay.  Assuming the fitrim code uses the free
> > space information to drive the discard calls, is it safe to do that with
> > unreplayed metadata?

Nop, not safe, neither with this patchset nor without it.
I've just sent a patch to do the same you did in your fixes for xfs and ext4:

https://patchwork.kernel.org/patch/10870871/

Thanks for reporting it!

>
> Pertinent question, indeed. But I'd defer to Filipe since he knows the
> log tree code. Filipe, FITRIM uses the freespace_ctl struct from block
> group to trim the freespace inside block groups, as well as the free
> device space to trim unallocated space. If we have a dirty log tree are
> those coherent with the dirty data i.e is it reflected in the BG's
> freespace cache that the data in the logs tree is actually allocated? If
> the answer is 'no' then it will be prudent to disallow trim in this case.
>
>
> >
> > (And no, I don't really know what nologreplay does, so please excuse my
> > ignorance...)
>
> Log replay means the content of the log tree (which is something like a
> WAL) must be copied back into the main btree.
>
> >
> > --D
> >
> >>   btrfs: combine device update operations during transaction commit
> >>   btrfs: Handle pending/pinned chunks before blockgroup relocation
> >>     during device shrink
> >>   btrfs: Rename and export clear_btree_io_tree
> >>   btrfs: Populate ->orig_block_len during read_one_chunk
> >>   btrfs: Introduce new bits for device allocation tree
> >>   btrfs: Remove 'trans' argument from find_free_dev_extent(_start)
> >>   btrfs: Factor out in_range macro
> >>   btrfs: Optimize unallocated chunks discard
> >>   btrfs: Implement find_first_clear_extent_bit
> >>   btrfs: Switch btrfs_trim_free_extents to find_first_clear_extent_bit
> >>
> >>  fs/btrfs/ctree.h            |   8 +-
> >>  fs/btrfs/dev-replace.c      |   2 +-
> >>  fs/btrfs/disk-io.c          |  20 ++-
> >>  fs/btrfs/extent-tree.c      | 102 +++++--------
> >>  fs/btrfs/extent_io.c        | 103 +++++++++++++-
> >>  fs/btrfs/extent_io.h        |  19 ++-
> >>  fs/btrfs/extent_map.c       |  38 +++++
> >>  fs/btrfs/extent_map.h       |   1 -
> >>  fs/btrfs/free-space-cache.c |   4 -
> >>  fs/btrfs/transaction.c      |  51 +------
> >>  fs/btrfs/transaction.h      |   2 +-
> >>  fs/btrfs/volumes.c          | 277 ++++++++++++++----------------------
> >>  fs/btrfs/volumes.h          |  23 ++-
> >>  13 files changed, 332 insertions(+), 318 deletions(-)
> >>
> >> --
> >> 2.17.1
> >>
> >



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Reply via email to