Re: [PATCH v2] vmpressure: make sure memcg stays alive until all users are signaled

[Tejun Heo <tj@kernel.org>]

Hello,

On Thu, Jul 11, 2013 at 06:22:15PM +0200, Michal Hocko wrote:
> I would rather not put vmpressure clean up code into memcg offlining.
> We have reference counting for exactly this purposes so it feels strange
> to overcome it like that.

It's not something white and black but for things which can be made
trivially synchrnous, it usually is better to do it that way,
especially while shutting down objects as they enter a lingering stage
where they're de-registered but not destroyed and you should be
careful which parts of the object are still accessible.  I haven't
read it carefully but here I'm not sure whether it's safe to do event
related operations after removal.  From cgroup core side, event list
is shut down synchronously from cgroup_destroy_locked().  It doesn't
seem like that part is explicitly built to remain accessible
afterwards.

> Besides that wouldn't be that racy? The work item could be already
> executing and preempted and we do not want vmpr to disappear from under
> our feet. I know this is _highly_ unlikely but why to play games?

Hmmm?  flush_work() and cancel_work_sync() guarantee that the work
item isn't executing on return unless it's being requeued.  There's no
race condition.

> Dunno, to be honest. From a quick look they both can be turned to spin
> locks but events_lock might cause long preempt disabled periods when
> zillions of events are registered.

I see.

> > Also, while at it, can you please remove the work_pending() check?
> > They're almost always spurious or racy and should be avoided in
> > general.
> 
> sure, this really looks bogus. I will cook patches for both issues and
> send them tomorrow.

Thanks.

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>