Re: [PATCH] sched: Move task_mm_cid_work to mm delayed work

[Mathieu Desnoyers]

On 2024-12-06 03:53, Gabriele Monaco wrote:
> On Thu, 2024-12-05 at 11:25 -0500, Mathieu Desnoyers wrote:
[...]
> > > The behaviour imposed by this patch (at least the intended one) is
> > > to
> > > run the task_mm_cid_work with the configured periodicity (plus
> > > scheduling latency) for each active mm.
> >
> > What you propose looks like a more robust design than running under
> > the tick.
> >
> > > This behaviour seem to me more predictable, but would that even be
> > > required for rseq or is it just an overkill?
> >
> > Your approach looks more robust, so I would be tempted to introduce
> > it as a fix. Is the space/runtime overhead similar between the
> > tick/task work approach vs yours ?
>
> I'm going to fix the implementation and come up with some runtime stats
> to compare the overhead of both methods.
> As for the space overhead, I think I can answer this question already:
> * The current approach uses a callback_head per thread (16 bytes)
> * Mine relies on a delayed work per mm (88 bytes)
>
> Tasks with 5 threads or less have lower memory footprint with the
> current approach.
> I checked quickly on some systems I have access to and I'd say my
> approach introduces some memory overhead on an average system, but
> considering a task_struct can be 7-13 kB and an mm_struct is about 1.4
> kB, the overhead should be acceptable.

ok!

> > > In other words, was the tick chosen out of simplicity or is there
> > > some
> > > property that has to be preserved?
> >
> > Out of simplicity, and "do like what NUMA has done". But I am not
> > particularly attached to it. :-)
> >
> > > P.S. I run the rseq self tests on both this and the previous patch
> > > (both broken) and saw no failure.
> >
> > That's expected, because the tests do not so much depend on the
> > compactness of the mm_cid allocation. They way I validated this
> > in the past is by creating a simple multi-threaded program that
> > periodically prints the current mm_cid from userspace, and
> > sleep for a few seconds between printing, from many threads on
> > a many-core system.
> >
> > Then see how it reacts when run: are the mm_cid close to 0, or
> > are there large values of mm_cid allocated without compaction
> > over time ? I have not found a good way to translate this into
> > an automated test though. Ideas are welcome.
> >
> > You can look at the librseq basic_test as a starting point. [1]
>
> Perfect, will try those!

Thinking back on this, you'll want a program that does the following
on a system with N CPUs:

- Phase 1: run one thread per cpu, pinned on each cpu. Print the
  mm_cid from each thread with the cpu number every second or so.

- Exit all threads except the main thread, join them from the main
  thread,

- Phase 2: the program is now single-threaded. We'd expect the
  mm_cid value to converge towards 0 as the periodic task clears
  unused CIDs.

So I think in phase 2 we can have an actual automated test: If after
an order of magnitude more time than the 100ms delay between periodic
tasks we still observe mm_cid > 0 in phase 2, then something is
wrong.

Thoughts ?

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com