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
