On 2013/7/15 17:20, Michal Hocko wrote:

On Mon 15-07-13 11:07:52, Li Zefan wrote:

quoted

On 2013/7/12 18:37, Michal Hocko wrote:

quoted

On Fri 12-07-13 17:54:27, Li Zefan wrote:

quoted

On 2013/7/12 17:29, Michal Hocko wrote:

quoted

On Fri 12-07-13 17:20:09, Li Zefan wrote:
[...]

quoted

But if I read the code correctly, even no one registers a vmpressure event,
vmpressure() is always running and queue the work item.

True but checking there is somebody is rather impractical. First we
would have to take a events_lock to check this and then drop it after
scheduling the work. Which doesn't guarantee that the registered event
wouldn't go away.
And even trickier, we would have to do the same for all parents up the
hierarchy.

The thing is, we can forget about eventfd. eventfd is checked in
vmpressure_work_fn(), while vmpressure() is always called no matter what.

But vmpressure is called only for an existing memcg. This means that
it cannot be called past css_offline so it must happen _before_ cgroup
eventfd cleanup code.

Or am I missing something?

Yeah.

The vmpressure work item is queued if we sense some memory pressure, no matter
if there is any eventfd ever registered. This is the point.

But it is queued on vmpr which is embedded in the memcg which is the
_target_ of the reclaim. There is _no reclaim_ for a memcg after css has
been deactivated which happens _before_ css_offline.

1. vmpressure() is called, and the work is queued.
2. then we rmdir cgroup, and struct mem_cgroup is freed finally.
3. workqueue schedules the work to run:

static void vmpressure_work_fn(struct work_struct *work)
{
        struct vmpressure *vmpr = work_to_vmpressure(work)
...

As vmpr is embeded in struct mem_cgroup, and memcg has been freed, this
leads to invalid memory access.

NOTE: no one ever registered an eventfd!