[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d25a9279-6687-df30-3ae1-93ad73bfe193@kernel.org>
Date: Wed, 1 Dec 2021 21:26:23 +0200
From: Georgi Djakov <djakov@...nel.org>
To: Johannes Weiner <hannes@...xchg.org>,
Georgi Djakov <quic_c_gdjako@...cinc.com>
Cc: vincent.guittot@...aro.org, juri.lelli@...hat.com,
peterz@...radead.org, mingo@...hat.com, rostedt@...dmis.org,
bsegall@...gle.com, mgorman@...e.de, bristot@...hat.com,
mhocko@...nel.org, vdavydov.dev@...il.com, tj@...nel.org,
axboe@...nel.dk, cgroups@...r.kernel.org,
linux-block@...r.kernel.org, akpm@...ux-foundation.org,
linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC] psi: Add additional PSI counters for each type of memory
pressure
On 10.11.21 18:44, Johannes Weiner wrote:
> On Wed, Nov 10, 2021 at 07:36:37AM -0800, Georgi Djakov wrote:
>> From: Carlos Ramirez <carlrami@...eaurora.org>
>>
>> Calculates psi totals for memory pressure subevents:
>> compaction, thrashing, direct compaction, direct reclaim, and kswapd0.
>> Uses upper 16 bits of psi_flags to track memory subevents.
>
> Oof, that's quite heavy both in terms of branches, but also in terms
> of cache - which, depending on wakeup pattern and cpu topology can
> really hurt those paths.
>
> What's the usecase? Do you have automation that needs to act on one
> type of stall but not the others, for example?
This is mostly for debugging and profiling purposes and does not have
any automation yet.
> I find that looking at vmstat events on hosts with elevated pressure
> tends to give a pretty good idea of the source. It should also be
> possible to whip up a short bpftrace script to track down culprit
> callstacks of psi_memstall_*.
I found very similar patchset that has been posted previously proposing
almost the same types and some tracepoints in addition to that. I don't
see anyone having an argument against this in the past, so I'm wondering
if this could be an acceptable approach?
https://lore.kernel.org/r/1585649077-10896-2-git-send-email-laoar.shao@gmail.com
>> @@ -1053,19 +1128,56 @@ int psi_show(struct seq_file *m, struct psi_group *group, enum psi_res res)
>> + seq_printf(m, "%s avg10=%lu.%02lu avg60=%lu.%02lu avg300=%lu.%02lu total=%llu %llu %llu %llu %llu %llu %llu %llu %llu %llu %llu %llu\n",
>> full ? "full" : "some",
>> LOAD_INT(avg[0]), LOAD_FRAC(avg[0]),
>> LOAD_INT(avg[1]), LOAD_FRAC(avg[1]),
>> LOAD_INT(avg[2]), LOAD_FRAC(avg[2]),
>> - total);
>> + total, total_blk_cgroup_throttle, total_bio, total_compaction,
>> + total_thrashing, total_cgroup_reclaim_high,
>> + total_cgroup_reclaim_high_sleep, total_cgroup_try_charge,
>> + total_direct_compaction, total_direct_reclaim, total_read_swappage,
>> + total_kswapd);
>
> The file format is a can of worms. I doubt we can change this at this
> point without breaking parsers, so those numbers would have to live
> somewhere else. But let's figure out the above questions before
> worrying about this.
Agree.
Thanks,
Georgi
Powered by blists - more mailing lists