I send: [PATCH] Btrfs: make should_defrag_range() understood compressed extents If you want you can test that, it fix btrfs fi def and autodefrag behavior with compressed data. Also it understood if user try recompress data with old/new compression algo. 2017-12-14 14:27 GMT+03:00 Timofey Titovets <nefelim4ag@xxxxxxxxx>: > 2017-12-14 8:58 GMT+03:00 Duncan <1i5t5.duncan@xxxxxxx>: >> Timofey Titovets posted on Thu, 14 Dec 2017 02:05:35 +0300 as excerpted: >> >>> Also, same problem exist for autodefrag case i.e.: >>> write 4KiB at start of compressed file autodefrag code add that file to >>> autodefrag queue, call btrfs_defrag_file, set range from start to u64-1. >>> That will trigger to full file rewrite, as all extents are smaller then >>> 256KiB. >>> >>> (if i understood all correctly). >> >> If so, it's rather ironic, because that's how I believed autodefrag to >> work, whole-file, for quite some time. Then I learned otherwise, but I >> always enable both autodefrag and compress=lzo on all my btrfs, so it >> looks like at least for my use-case, I was correct with the whole-file >> assumption after all. (Well, at least for files that are actually >> compressed, I don't run compress-force=lzo, just compress=lzo, so a >> reasonable number of files aren't actually compressed anyway, and they'd >> do the partial-file rewrite I had learned to be normal.) >> >> -- >> Duncan - List replies preferred. No HTML msgs. >> "Every nonfree program has a lord, a master -- >> and if you use the program, he is your master." Richard Stallman >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > btrfs fi def can easy avoid that problem if properly documented, > but i think that fix must be in place, because "full rewrite" by > autodefrag are unacceptable behaviour. > > How i see, "How it works": > Both, defrag ioctl and autodefrag - call btrfs_defrag_file() (ioctl.c). > btrfs_defrag_file() set extent_thresh to args, if args not provided > (autodefrag not initialize range->extent_thresh); > use default: > if (extent_thresh == 0) > extent_thresh = SZ_256K; > > Later btrfs_defrag_file() try defrag file from start by "index" (page > number from start, file virtually splitted to page sized blocks). > Than it call should_defrag_range(), if need make i+1 or skip range by > info from should_defrag_range(). > > should_defrag_range() get extent for specified start offset: > em = defrag_lookup_extent(inode, start); > > Later (em->len >= thresh || (!next_mergeable && !prev_mergeable)) will > fail condition because len (<128KiB) < thresh (256KiB or 32MiB usual). > So extent will be rewritten. > > struct extent_map{}; have two potential useful info: > > ... > u64 len; > ... > > unsigned int compress_type; > ... > > As i see by code len store "real" length, so > in theory that just need to add additional check later like: > if (em->len = BTRFS_MAX_UNCOMPRESSED && em->compress_type > 0) > ret = 0; > > That must fix problem by "true" insight check for compressed extents. > > Thanks. > > P.S. > (May be someone from more experienced devs can comment that?) > > -- > Have a nice day, > Timofey. -- Have a nice day, Timofey. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
