> On Nov. 5, 2015, 1:14 a.m., Matthew Ahrens wrote: > > usr/src/uts/common/fs/zfs/arc.c, line 1029 > > <https://reviews.csiden.org/r/267/diff/3/?file=18198#file18198line1029> > > > > It would be reasonable to not handle byte swapping - if you move the > > pool (& l2arc) to a different endian, you can deal with losing the contents > > of the l2arc. Not sure if it saves us much code but definitely saves > > testing! > > Saso Kiselkov wrote: > It literally saves two lines. At such a low line count, I'd wager even I > can write bug-free code. Yes, this might rarely be used, but this might end > up hurting portability especially on the rarer platforms (not necessarily > Illumos-supported ones - think Linux).
OK > On Nov. 5, 2015, 1:14 a.m., Matthew Ahrens wrote: > > usr/src/uts/common/fs/zfs/arc.c, line 1031 > > <https://reviews.csiden.org/r/267/diff/3/?file=18198#file18198line1031> > > > > should we add a version field (string?) here so that we can easily > > change the l2arc on-disk format and detect that we need to start over? Or > > did you have an alternate plan for how we will do that? > > Saso Kiselkov wrote: > I had originally planned to use feature flags and/or changing the magic, > but if you insist, I'll add a field. Changing the magic seems like an OK strategy. I'm not going to insist. > On Nov. 5, 2015, 1:14 a.m., Matthew Ahrens wrote: > > usr/src/uts/common/fs/zfs/arc.c, line 1069 > > <https://reviews.csiden.org/r/267/diff/3/?file=18198#file18198line1069> > > > > Given that it isn't a big deal to wipe the l2arc and start over, do we > > really need to reserve padding for future expansion? Seems like we can > > just change this and lose the contents of the l2arc once. > > Saso Kiselkov wrote: > Actually, with persistent L2ARC, it is becoming quite a deal to lose it, > since it can significantly affect performance (especially considering we plan > on offloading most DDT reading on there). Another feature we plan to > implement is mirrored L2ARC - this is to just underscore how important it is > keeping the L2ARC around. > So I disagree that it isn't a big deal. Sometimes there's no way around > it, but I'd like to minimize those cases. To that end, we could add a version > field in here as well (or use my originally planned mechanism, change magic). OK > On Nov. 5, 2015, 1:14 a.m., Matthew Ahrens wrote: > > usr/src/uts/common/fs/zfs/arc.c, lines 1092-1096 > > <https://reviews.csiden.org/r/267/diff/3/?file=18198#file18198line1092> > > > > this still works OK with the new SPA_MAXBLOCKSIZE of 16MB? > > Saso Kiselkov wrote: > It does, but pushes the minimum size limit of rebuildable L2ARC devices > up to ~51 GB. With the old SPA_MAXBLOCKSIZE the limit was 400 MB. I can try > to make this switchable based on whether largeblock support is used on the > pool. But I'll think about whether this limit makes sense anyhow. I basically > only put it in to avoid people testing it on laughably small L2ARC devices > (e.g. loopback to a 128mb file). Personally, I'm fine with a lower limit of ~50GB. > On Nov. 5, 2015, 1:14 a.m., Matthew Ahrens wrote: > > usr/src/uts/common/fs/zfs/arc.c, line 5800 > > <https://reviews.csiden.org/r/267/diff/3/?file=18198#file18198line5800> > > > > When we reconstruct, are we able to set the write head back to > > approximately where it was before reboot? > > Saso Kiselkov wrote: > Yes we do, it's in l2arc_rebuild(). We set the write hand to the next > block following the last log block. cool. - Matthew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.csiden.org/r/267/#review867 ----------------------------------------------------------- On Nov. 5, 2015, 3:31 p.m., Saso Kiselkov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.csiden.org/r/267/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2015, 3:31 p.m.) > > > Review request for OpenZFS Developer Mailing List and Christopher Siden. > > > Repository: illumos-gate > > > Description > ------- > > This is an upstream port of the Persistent L2ARC feature from Nexenta's repo. > > > Diffs > ----- > > usr/src/uts/common/sys/fs/zfs.h bc9f057dd1361ae73a12375515abacd0fed820d2 > usr/src/uts/common/fs/zfs/vdev_label.c > f0924ab1e66eaa678540da8925995da6e0e2a29c > usr/src/uts/common/fs/zfs/vdev.c 1c57fce4dcee909b164353181dcd8e2a29ed7946 > usr/src/uts/common/fs/zfs/sys/spa.h > 7ac78390338a44f7b7658017e1ae8fcc9beb89d6 > usr/src/uts/common/fs/zfs/sys/arc.h > 899b72114b9909098080a5d6bbad1a60808f030c > usr/src/uts/common/fs/zfs/spa.c 95a6b0fae7760e8a1e8cfc1e657dc22fd9ef3245 > usr/src/uts/common/fs/zfs/arc.c 52f582be1633a8d462105e068ae9679c04753d24 > usr/src/lib/libzpool/common/sys/zfs_context.h > 9e4d8ed0b8ec42be75bb93f44602ac99e907cf00 > > Diff: https://reviews.csiden.org/r/267/diff/ > > > Testing > ------- > > Been running in Nexenta's QA process for the past 2+ months. > > > Thanks, > > Saso Kiselkov > >
_______________________________________________ developer mailing list developer@open-zfs.org http://lists.open-zfs.org/mailman/listinfo/developer
