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 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.
--
Andriy Gapon
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer
