Re: [PATCH 2/2] mm: Consider subtrees in memory.events

From: Tejun Heo
Date: Mon Jan 28 2019 - 10:41:57 EST


Hello, Michal.

On Mon, Jan 28, 2019 at 04:18:59PM +0100, Michal Hocko wrote:
> How do you make an atomic snapshot of the hierarchy state? Or you do
> not need it because event counters are monotonic and you are willing to
> sacrifice some lost or misinterpreted events? For example, you receive
> an oom event while the two children increase the oom event counter. How
> do you tell which one was the source of the event and which one is still
> pending? Or is the ordering unimportant in general?

Hmm... This is straightforward stateful notification. Imagine the
following hierarchy. The numbers are the notification counters.

A:0
/ \
B:0 C:0

Let's say B generates an event, soon followed by C. If A's counter is
read after both B and C's events, nothing is missed.

Let's say it ends up generating two notifications and we end up
walking down inbetween B and C's events. It would look like the
following.

A:1
/ \
B:1 C:0

We first see A's 0 -> 1 and then start scanning the subtrees to find
out the origin. We will notice B but let's say we visit C before C's
event gets registered (otherwise, nothing is missed).

But, no matter where you put C's event and notification, the
followings hold.

1. A's count will be different from what was seen before.
2. There will be another notification queued on A.

IOW, it's guaranteed that we'll notice and re-scan if we don't see C's
event this time. The worst that can happen is scanning spuriously but
that's true even for local events.

This isn't a novel thing. It's how aggregated stateful notifications
usually work (e.g. a lot of hardware interrupts behave this way). The
notification is just saying "something might have changed here, please
take a look" and the interlocking is achieved by following specific
orders when propagating and reading the events.

> I can imagine you can live with this model, but having a hierarchical
> reporting without a source of the event just sounds too clumsy from my
> POV. But I guess this is getting tangent to the original patch.

It seems like your opinion is mostly based on misunderstanding. Let's
keep the discussion focused on API stability.

Thanks.

--
tejun