On Fri, Jun 16, 2023 at 12:31:44PM -0700, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 16 Jun 2023 20:07:18 +0200 Michal Koutný <mkoutny@xxxxxxxx> wrote: > > > An issue was observed with stats collected in struct rusage on ppc64le > > with 64kB pages. The percpu counters use batching with > > percpu_counter_batch = max(32, nr*2) # in PAGE_SIZE > > i.e. with larger pages but similar RSS consumption (bytes), there'll be > > less flushes and error more noticeable. > > A fully detailed description of the issue would be helpful. Obviously > "inaccuracy", but how bad? Any reader of get_mm_counter() could see the inaccuracy given by the formula. In this particular case it is detected by a testsuite of time(1) utility that feeds from rusage: > FAIL: tests/time-max-rss > ======================== > > time(1) failed to detect 5MB allcoation. > mem-baseline(kb): 0 > mem-5MB(kb): 4096 > delta(kb): 4096 > FAIL tests/time-max-rss.sh (exit status: 1) (i.e. 1MB missing) > Far too large to be inlined! For six callsites it adds 1kb of text. Ah, thanks, I can change that. > Why even modify the counter? Can't <whatever this issue is> be > addressed by using percpu_counter_sum() in an appropriate place? I considered modifying get_mm_counter(), however, I decided not to put the per-cpu summing there as it'd incur the impact to many more places than sync_mm_rss(). > For unknown reasons percpu_counter_set() uses for_each_possible_cpu(). > Probably just a mistake - percpu_counters are hotplug-aware and > for_each_online_cpu should suffice. Yeah, that could be cleaned up in another patch (cf mask in __percpu_counter_sum). > I'm really not liking percpu_counter_set(). It's only safe in > situations where the caller knows that no other CPU can be modifying > the counter. I wonder if all the callers know that. I admit I only considered the do_exit() path (and even that isn't granted in a multithreaded process) -- so I don't like percpu_counter_set() in this current form neither. I will need to review effects of parallel updates more. Thanks, Michal
Attachment:
signature.asc
Description: PGP signature
