On Wed, Jan 25, 2023 at 08:06:46AM -0300, Leonardo Brás wrote: > On Wed, 2023-01-25 at 09:33 +0100, Michal Hocko wrote: > > On Wed 25-01-23 04:34:57, Leonardo Bras wrote: > > > Disclaimer: > > > a - The cover letter got bigger than expected, so I had to split it in > > > sections to better organize myself. I am not very confortable with it. > > > b - Performance numbers below did not include patch 5/5 (Remove flags > > > from memcg_stock_pcp), which could further improve performance for > > > drain_all_stock(), but I could only notice the optimization at the > > > last minute. > > > > > > > > > 0 - Motivation: > > > On current codebase, when drain_all_stock() is ran, it will schedule a > > > drain_local_stock() for each cpu that has a percpu stock associated with a > > > descendant of a given root_memcg. > > > > > > This happens even on 'isolated cpus', a feature commonly used on workloads that > > > are sensitive to interruption and context switching such as vRAN and Industrial > > > Control Systems. > > > > > > Since this scheduling behavior is a problem to those workloads, the proposal is > > > to replace the current local_lock + schedule_work_on() solution with a per-cpu > > > spinlock. > > > > If IIRC we have also discussed that isolated CPUs can simply opt out > > from the pcp caching and therefore the problem would be avoided > > altogether without changes to the locking scheme. I do not see anything > > regarding that in this submission. Could you elaborate why you have > > abandoned this option? > > Hello Michal, > > I understand pcp caching is a nice to have. > So while I kept the idea of disabling pcp caching in mind as an option, I first > tried to understand what kind of impacts we would be seeing when trying to > change the locking scheme. Remote draining reduces interruptions whether CPU is marked as isolated or not: - Allows isolated CPUs from benefiting of pcp caching. - Removes the interruption to non isolated CPUs. See for example https://lkml.org/lkml/2022/6/13/2769 "Minchan Kim tested this independently and reported; My workload is not NOHZ CPUs but run apps under heavy memory pressure so they goes to direct reclaim and be stuck on drain_all_pages until work on workqueue run. unit: nanosecond max(dur) avg(dur) count(dur) 166713013 487511.77786438033 1283 From traces, system encountered the drain_all_pages 1283 times and worst case was 166ms and avg was 487us. The other problem was alloc_contig_range in CMA. The PCP draining takes several hundred millisecond sometimes though there is no memory pressure or a few of pages to be migrated out but CPU were fully booked. Your patch perfectly removed those wasted time." > After I raised the data in the cover letter, I found that the performance impact > appears not be that big. So in order to try keeping the pcp cache on isolated > cpus active, I decided to focus effort on the locking scheme change. > > I mean, my rationale is: if is there a non-expensive way of keeping the feature, > why should we abandon it? > > Best regards, > Leo > > > > > > > >
