Aaaah. I knew I had thought about something like this before. But when I couldn't find the code changes I figured I was hallucinating. Glad to know that my sanity is intact. Changes still look good to me.
--matt On Tue, Jan 14, 2014 at 9:57 AM, Saso Kiselkov <[email protected]>wrote: > On 1/14/14, 11:51 AM, Andriy Gapon wrote: > > > > Recently I noticed that on one of our systems l2arc_feed_thread was > consuming > > non insignificant amount of CPU time. I looked at some stats (FreeBSD > has local > > changes that add a few kstats for L2ARC) and it seems that > > the thread was busy scanning and re-scanning buffers that are already in > L2 ARC. > > Buffers were being skipped because l2arc_write_eligible was returning > false > > because of ab->b_l2hdr != NULL. As far as I can see the thread was > doing a pass > > per second and it typically scanned about 2 million buffers per pass. > Typically > > walking over a buffer list was aborted due to passed_sz > headroom. > > > > The system in question has a quite large ARC with maximum size of 60GB. > 26GB > > were actually in use and it seems that most of the buffers were rather > small, > > hash_elements == 3634055. > > > > Perhaps, there could be some optimization to avoid pointless walking over > > millions of buffers in situations like this? > > > > P.S. Because of another local FreeBSD change the feed thread was > scanning about > > 16 times more buffers than it would on illumos, so the issue was more > prominent > > with the thread consuming about 40% of a core. > > That leak should have been fixed... yep found the webrev: > http://cr.illumos.org/~webrev/skiselkov/3995/ > Unfortunately, it appears I dropped the ball on this one and forgot to > submit an RTI for it. > > Could people please give me a quick ok on the above webrev again? I've > updated it in place. All it really does is move the > l2arc_release_cdata_buf step in front of the mutex_trylock - since the > b_tmp_cdata pointer is thread-local, we don't need to grab locks it to > manipulate it. The rest of the changes are just renaming 'abl2' to > 'l2hdr' to be consistent across all of arc.c. > > Cheers, > -- > Saso > _______________________________________________ > developer mailing list > [email protected] > http://lists.open-zfs.org/mailman/listinfo/developer >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
