Changes look good to me. Saso, can you bring these over to illumos? --matt
On Mon, Mar 3, 2014 at 1:43 AM, Andriy Gapon <[email protected]> wrote: > on 20/02/2014 23:19 Matthew Ahrens said the following: > > line 4916, we can't just assert that b_tmp_cdata==NULL? I.e. there are > cases > > where it is non-NULL but we should ignore it because b_compress==OFF? > Should we > > be initializing b_tmp_cdata to NULL to prevent that? > > b_tmp_cdata points to original data in the case when no compression is > done. > On your advice I changed the code so that b_tmp_cdata is NULL in that case. > > > The code in arc_buf_l2_cdata_free() and arc_buf_data_free() is very > similar. I > > think we should be able to reuse some of that code, e.g. do something > like: > > Yes, the code is quite similar and it makes sense to factor out the common > code. > I have done that slightly differently from your suggestion, so that I > could keep > l2_cdata_free_on_write counter. But I am not sure how useful that counter > actually is to the general userbase. It was useful for me to verify that > the > code works. > > The new patch: > > https://github.com/avg-I/freebsd/compare/master...wip;hc;l2arc-compression-memory-leak.diff > > https://github.com/avg-I/freebsd/compare/master...wip;hc;l2arc-compression-memory-leak > > Please note that github seems to have a bug where '@@' lines of a diff have > incorrect function names. > > > On Thu, Feb 20, 2014 at 3:05 AM, Andriy Gapon <[email protected] > > <mailto:[email protected]>> wrote: > > > > > > To recap: after reports from FreeBSD ZFS users, I was able to > capture two types > > of l2arc temporary compressions buffer leaks. > > One was in arc_release() called from dbuf_dirty(). > > The other was in arc_hdr_destroy() called from arc_write_done() in > the > > ZIO_FLAG_IO_REWRITE case. > > In both case the mentioned calls happened concurrently with l2arc > writes and the > > corresponding buffers were in ARC_L2_WRITING state. > > Perhaps there were more cases... > > > > So, here is my current patch for the issue: > > > https://github.com/avg-I/freebsd/compare/master...wip;hc;l2arc-compression-memory-leak.diff > > Drop .diff to get a github branch comparison interface for more > details, if > > wanted. > > > > An earlier version of this patch has already been successfully > tested by the > > original reporters. The patch also survives HybridCluster testing > without any > > leaks or triggered assertions. > > > > Please review and comment. > > Thank you! > > -- > > Andriy Gapon > > > > > > > -- > Andriy Gapon >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
