On Wed, Jan 29, 2014 at 7:32 AM, Andriy Gapon <[email protected]> wrote:
> on 14/01/2014 19:33 Andriy Gapon said the following: > > I was worried about buffers that get put on l2arc_free_on_write list in > > arc_buf_data_free() in the case when an L2 write is still ongoing. More > > specifically, it seems that arc_hdr_destroy() would remove an > arc_buf_hdr_t > > object from l2ad_buflist if it's there before calling > arc_buf_data_free(). And > > thus l2arc_write_done() won't see that buffer. > > I am more and more of opinion that L2ARC compression commit introduced a > memory > leak for compressed data buffers in arc_hdr_destroy. > > The code in arc_hdr_destroy destroys b_l2hdr header, but the actual data is > still pointed to by b_buf. arc_hdr_destroy then calls arc_buf_destroy, > which > uses arc_buf_data_free to free the data. The data can either be freed > immediately or, if the data is still in use by an L2ARC write zio, it can > be put > on l2arc_free_on_write list, so that it is freed later when the zio is > done. > > The code seems to be unchanged by the L2ARC compression commit and the > code is > sound as long as there is no compression. > But if L2ARC compression is enabled, then in addition to the original data > buffer we now also have a buffer with compressed data. That buffer is > pointed > to by b_tmp_cdata in l2arc_buf_hdr_t. > There has to be some additional code to free the compressed buffer in the > arc_hdr_destroy code path described above. b_tmp_cdata must be passed > along > with b_data via l2arc_free_on_write list and b_tmp_cdata must then be > freed in > l2arc_do_free_on_write. > > My understanding was that the b_tmp_cdata should be NULL when arc_hdr_destroy() is called. It should have previously been freed and NULL-ed out in l2arc_release_cdata_buf(). I would suggest that we add assertions in arc_hdr_destroy() that b_tmp_cdata is NULL. Also, in l2arc_release_cdata_buf() b_tmp_cdata should be NULL if b_compress != LZ4. --matt
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
