Fix inaccurate arcstat_l2_hdr_size calculations #1522
Conversation
Based on the comments in arc.c we know that buffers can exist both in arc and l2arc, under this circumstance both arc_buf_hdr_t and l2arc_buf_hdr_t will be allocated. However the current logic only cares for memory that l2arc_buf_hdr takes up when the buffer's state transfers from or to arc_l2c_only. This will cause obvious deviations for illumos's zfs version since the sizeof(l2arc_buf_hdr) is larger than ZOL's. We can implement the calcuation in the following simple way: 1. When allocate a l2arc_buf_hdr_t we add its memory consumption instantly and subtract it when we free or evict the l2arc buf. 2. According to the code in l2arc_hdr_stat_add and l2arc_hdr_stat_remove, if the buffer only stays in l2arc we should also add the memory its arc_buf_hdr_t consumes, so we only need to add HDR_SIZE to arcstat_l2_hdr_size since we already concerned with L2HDR_SIZE in step 1 and vice vesa. Signed-off-by: Ying Zhu <casualfisher@gmail.com>
|
@casualfish Thanks for the patch, we'll review it carefully when we get a little time. Perhaps you can run it on your system to verify it's working as you expect. Upon first inspection I do see a few things which don't appear quite right to me so perhaps you can explain. Comments inline. |
|
@behlendorf Thanks for your team's code review, please feel free to point out if anything is inappropriate. I'll paste my test results here when I gather enough information next week. For your inline comments my intention was to use arc_space_* instead of manually operating on arcstat_data_size, but I missed to notice the addition and subtraction of arc_meta_used in arc_space_*, since we only care for data arc bufs occpy here we should not mess arc_size and arc_meta_used up. I'll reshape my patch and bring test results here in a new pull request, so close this pull request now. |
Based on the comments in arc.c we know that buffers can exist both in arc
and l2arc, under this circumstance both arc_buf_hdr_t and l2arc_buf_hdr_t
will be allocated. However the current logic only cares for memory that
l2arc_buf_hdr takes up when the buffer's state transfers from or to
arc_l2c_only. This will cause obvious deviations for illumos's zfs version
since the sizeof(l2arc_buf_hdr) is larger than ZOL's. We can implement
the calcuation in the following simple way:
and subtract it when we free or evict the l2arc buf.
if the buffer only stays in l2arc we should also add the memory its arc_buf_hdr_t
consumes, so we only need to add HDR_SIZE to arcstat_l2_hdr_size since we already
concerned with L2HDR_SIZE in step 1 and vice vesa.
Signed-off-by: Ying Zhu casualfisher@gmail.com