Any progress on this? --matt
On Wed, Jun 11, 2014 at 9:03 AM, Matthew Ahrens <[email protected]> wrote: > Changes look good to me. > > I would just request that you add a comment in spa_sync_upgrades() > explaining that the LZ4 feature wasn't always activate_on_enable, so it > could be enabled but not yet active. Otherwise someone might come along > this code and think it is not necessary because LZ4 is activate_on_enable. > > Once that is done, feel free to file an RTI and count me as a reviewer. > > --matt > > > On Wed, Jun 11, 2014 at 5:56 AM, Daniil Lunev <[email protected]> > wrote: > >> Updated patch to the LZ4 metadata compression >> http://cr.illumos.org/~webrev/DKOIman/LZ4_MD/ >> ZDB output is attached. Not so big gain on compression rate, though, it >> is better. Also, it was shown in initial lz4 commit that lz4 is faster. >> That's why changing metadata compression to the lz4 aglorithm is a >> reasonable idea. >> --Daniil Lunev. >> >> >> On Wed, Jun 11, 2014 at 9:46 AM, Matthew Ahrens <[email protected]> >> wrote: >> >>> resend to [email protected], where an alternate patch was posted. >>> >>> --matt >>> >>> >>> On Tue, Jun 10, 2014 at 7:23 PM, Matthew Ahrens <[email protected]> >>> wrote: >>> >>>> This looks generally good, but I'm concerned about enabling the feature >>>> from this code path. I think it will work, but it doesn't really have the >>>> right locks to ensure that it will continue to work. I think >>>> dmu_write_policy() should just check if the property is already active. We >>>> could add a separate routine to ensure that lz4 is always active. >>>> Specifically: >>>> >>>> 1. Register the LZ4_COMPRESS feature with >>>> zfeature_register(activate_on_enable==B_TRUE). >>>> 2. To deal with pools that already have the feature enabled but not >>>> active, activate the feature if it is enabled but not yet active. >>>> spa_sync_upgrades() would be an appropriate place to do this. >>>> 3. Don't bother activating the feature when doing "zfs set >>>> compression=lz4" (i.e. remove the ZFS_PROP_COMPRESSION case from >>>> zfs_prop_set_special(); remove zfs_prop_activate_feature{_sync,_check}) >>>> 4. Update the zpool-features.5 manpage to indicate that the feature >>>> will be activated when enabled. See the documentation of the birth_hole >>>> feature for example wording to copy. >>>> >>>> --matt >>>> >>>> >>>> On Tue, Jun 10, 2014 at 12:34 PM, Daniil Lunev via illumos-developer < >>>> [email protected]> wrote: >>>> >>>>> Hello, all. >>>>> As the lz4 algorithm is better than other we have in illumos. Though, >>>>> we still use lzjb to compress metadata. Here is the patch to use lz4 for >>>>> the metadata when the corresponing feature is enabled. >>>>> http://cr.illumos.org/~webrev/DKOIman/LZ4_MD/ Mail_msg is attached, >>>>> Thanks, >>>>> Daniil Lunev >>>>> P.S. Sorry if it's duplicate - somehow first mail didn't go. >>>>> *illumos-developer* | Archives >>>>> <https://www.listbox.com/member/archive/182179/=now> >>>>> <https://www.listbox.com/member/archive/rss/182179/21175174-cd73734d> >>>>> | Modify >>>>> <https://www.listbox.com/member/?member_id=21175174&id_secret=21175174-792643f6> >>>>> Your Subscription <http://www.listbox.com> >>>>> >>>> >>>> >>> >> >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
