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