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.

-- 
Andriy Gapon
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer

Reply via email to