On Thu, Sep 18, 2014 at 1:48 AM, Andriy Gapon <[email protected]> wrote:
> On 17/09/2014 23:52, Matthew Ahrens wrote: > > > > > > On Wed, Sep 17, 2014 at 1:08 PM, Andriy Gapon <[email protected] > > <mailto:[email protected]>> wrote: > > > > > > It seems that dead code removing is a hot topic :-) > > I think that the following commit has introduced a little bit of > such code: > > commit 5d7b4d438c4a51eccc95e77a83a437b4d48380eb > > Author: Matthew Ahrens <[email protected] <mailto: > [email protected]>> > > Date: Thu Jun 5 13:19:08 2014 -0800 > > > > 4757 ZFS embedded-data block pointers ("zero block compression") > > 4913 zfs release should not be subject to space checks > > Reviewed by: [many] > > > > In the following snippet psize == lsize condition is taken care of > in the first > > if-block, so that condition can never be true within the else-block. > > > > > > That's not true. When we check psize==lsize in the "else" block, psize > may not > > be the same value that was checked earlier. It is modified 2 lines > above, to > > round it up to a multiple of SPA_MINBLOCKSIZE. > > Ah, sorry for the noise, I overlooked the roundup. > > > + /* > > + * Round up compressed size to MINBLOCKSIZE > and > > + * zero the tail. > > + */ > > + size_t rounded = > > + P2ROUNDUP(psize, > (size_t)SPA_MINBLOCKSIZE); > > + if (rounded > psize) { > > + bzero((char *)cbuf + psize, rounded > - psize); > > + psize = rounded; > > + } > > + if (psize == lsize) { > > + compress = ZIO_COMPRESS_OFF; > > + zio_buf_free(cbuf, lsize); > > Last note, I think that it would be logical to assert that rounded is not > greater than lsize Sure. > and also move bzero() calls to be after a check that rounded > == lsize. Otherwise, in a case when psize < rounded && rounded == lsize > we are > calling bzero() on a buffer that we subsequently discard. > I doubt that this matters for performance. We should aim for the code to be as clear as possible. If you can rewrite this logic to be more clear, I'd welcome the patch. --matt
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
