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