----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.csiden.org/r/266/#review865 -----------------------------------------------------------
usr/src/man/man1m/zfs.1m (line 1034) <https://reviews.csiden.org/r/266/#comment642> I think we are supposed to add only mandoc-style markup. usr/src/man/man1m/zfs.1m (line 1057) <https://reviews.csiden.org/r/266/#comment643> Changing the default seems a little bold to me, if this can cause reduced compression ratio in some circumstances. What would you think about integrating with the default being off, and then coming back in ~6 months after this has gotten wider exposure? Kind of like what we did for lz4, but hopefully waiting less long before making it the default :) Alternatively, maybe you have some data about how on by default won't be that bad? usr/src/man/man1m/zfs.1m (line 1063) <https://reviews.csiden.org/r/266/#comment644> Looks like you've inadvertently reverted this from the mandoc-style to the old, roff style. usr/src/uts/common/fs/zfs/sys/zfs_znode.h (line 215) <https://reviews.csiden.org/r/266/#comment647> I think the recommended style is one struct member per line. usr/src/uts/common/fs/zfs/zfs_vnops.c (lines 860 - 861) <https://reviews.csiden.org/r/266/#comment645> wow, I forgot about that functionality. I guess you're glad we didn't rip out that dead code :-) usr/src/uts/common/fs/zfs/zfs_znode.c (lines 703 - 714) <https://reviews.csiden.org/r/266/#comment646> Did you consider implementing the smartcomp stuff at the DMU layer, rather than the ZPL? It seems like that might simplify the implementation, because you wouldn't need special callbacks for this, and the results_cb() could go away and we could do its logic in dbuf_write_done(). You might also be able to eliminate the ask_cb, by having the DMU set the zio_prop_t appropriately (in dmu_write_policy). Additinally, it seems like the smartcomp logic could also apply to zvols (maybe you write some compressable data, and then some uncompressable data), and to other file-type objsets (e.g. if Lustre uses its own DMU consumer?) In any case, is there any ZPL-specific logic? So far I haven't seen it trying to do something like look at the filename extension or other ZPL metadata to make a determination. - Matthew Ahrens On Oct. 28, 2015, 9:25 p.m., Saso Kiselkov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.csiden.org/r/266/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2015, 9:25 p.m.) > > > Review request for OpenZFS Developer Mailing List and Christopher Siden. > > > Repository: illumos-gate > > > Description > ------- > > ZFS smart compression is a feature developed at Nexenta that automatically > tracks per-file compression ratios and disables compression if a file is > consistently getting too low a compression ratio to warrant further attempts > compressing it. The will periodically recheck its previous conclusions about > the (in)compressibility of a file using a progressive weighting algorithm. > The more compression succeeds, the more hesitant the algorithm is to stop > attempting it in case a short incompressible transient is encountered and > conversely, the more compression attempts fail, the less frequent the > algorithm rechecks (up to some meaningful limits). > > > Diffs > ----- > > usr/src/uts/common/sys/fs/zfs.h bc9f057dd1361ae73a12375515abacd0fed820d2 > usr/src/uts/common/fs/zfs/sys/zio.h > 877e2839bf804ca3625c907367fed4a06744ea52 > usr/src/uts/common/fs/zfs/sys/zfs_znode.h > df08aad7b41e3648a5c5f493e3dd1d83390a4c40 > usr/src/uts/common/fs/zfs/sys/dmu_tx.h > 4d4253a4ec505e835bd87424f68c7e3eae7d9e3e > usr/src/uts/common/fs/zfs/sys/dmu_objset.h > 8a263a39e6b30b644742a31caa32e973dcfb9224 > usr/src/uts/common/fs/zfs/sys/dbuf.h > 233d541342ad8b80781d44c62d3a768f453b82ea > usr/src/uts/common/fs/zfs/sys/arc.h > 899b72114b9909098080a5d6bbad1a60808f030c > usr/src/uts/common/fs/zfs/zio.c 4a3dafac9f7f391a9f16c06932420f655dde61f0 > usr/src/uts/common/fs/zfs/zfs_znode.c > 1321508164dd4fbfa12885cebca80db691ebd64f > usr/src/uts/common/fs/zfs/zfs_vnops.c > 49b158764036b155060d57c5953bf9e2a7572dc4 > usr/src/uts/common/fs/zfs/dmu_tx.c 50885acafc9c83bedcc6e902b6528622fd5a3034 > usr/src/uts/common/fs/zfs/dmu_objset.c > 79de1d127d426fe3c546a03e4a5d0e533b7672e6 > usr/src/uts/common/fs/zfs/dmu.c ceb08e227fd06bf55363b410129549eb8b3c7f0e > usr/src/uts/common/fs/zfs/dbuf.c 8b0c71db5c86fab2219699f4e3b2eccf00de545c > usr/src/uts/common/fs/zfs/arc.c 52f582be1633a8d462105e068ae9679c04753d24 > usr/src/man/man1m/zfs.1m 328fdcca3a657a00b32121f8334276fd162a01da > usr/src/common/zfs/zfs_prop.c 0d486708ae93e61551bb9eea8c58a600925c8a9c > > Diff: https://reviews.csiden.org/r/266/diff/ > > > Testing > ------- > > Been tested in Nexenta's internal QA for the past month. > > > Thanks, > > Saso Kiselkov > >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
