From: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
To: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>
Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Anton Vorontsov
<anton.vorontsov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>,
KAMEZAWA Hiroyuki
<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>,
KOSAKI Motohiro
<kosaki.motohiro-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
Subject: Re: [PATCH v2] vmpressure: make sure memcg stays alive until all users are signaled
Date: Fri, 12 Jul 2013 17:20:09 +0800 [thread overview]
Message-ID: <51DFCA49.4080407@huawei.com> (raw)
In-Reply-To: <20130712084039.GA13224-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
>> 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.
>
> /me goes and checks the code
>
> vmpressure_event sends signals to _registered_ events but those are
> unregistered from the work queue context by cgroup_event_remove (via
> vmpressure_unregister_event) queued from cgroup_destroy_locked.
>
> I am not sure what are the guarantees for ordering on the workqueue but
> this all suggests that either vmpressure_event sees an empty vmpr->events
> or it can safely send signals as cgroup_event_remove is pending on the
> queue.
>
> cgroup_event_remove drops a reference to cgrp->dentry after everything
> is unregistered and event->wait removed from the wait queue so
> cgroup_free_fn couldn't have been called yet and so memcg is still
> alive. This means that even css_get/put is not necessary.
>
> So I guess we are safe with the code as is but this all is really
> _tricky_ and deserves a fat comment. So rather than adding flushing work
> item code we should document it properly.
>
> Or am I missing something?
>
But if I read the code correctly, even no one registers a vmpressure event,
vmpressure() is always running and queue the work item.
WARNING: multiple messages have this Message-ID (diff)
From: Li Zefan <lizefan@huawei.com>
To: Michal Hocko <mhocko@suse.cz>
Cc: Tejun Heo <tj@kernel.org>,
Anton Vorontsov <anton.vorontsov@linaro.org>,
cgroups@vger.kernel.org, linux-mm@kvack.org,
Johannes Weiner <hannes@cmpxchg.org>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Subject: Re: [PATCH v2] vmpressure: make sure memcg stays alive until all users are signaled
Date: Fri, 12 Jul 2013 17:20:09 +0800 [thread overview]
Message-ID: <51DFCA49.4080407@huawei.com> (raw)
In-Reply-To: <20130712084039.GA13224@dhcp22.suse.cz>
>> 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.
>
> /me goes and checks the code
>
> vmpressure_event sends signals to _registered_ events but those are
> unregistered from the work queue context by cgroup_event_remove (via
> vmpressure_unregister_event) queued from cgroup_destroy_locked.
>
> I am not sure what are the guarantees for ordering on the workqueue but
> this all suggests that either vmpressure_event sees an empty vmpr->events
> or it can safely send signals as cgroup_event_remove is pending on the
> queue.
>
> cgroup_event_remove drops a reference to cgrp->dentry after everything
> is unregistered and event->wait removed from the wait queue so
> cgroup_free_fn couldn't have been called yet and so memcg is still
> alive. This means that even css_get/put is not necessary.
>
> So I guess we are safe with the code as is but this all is really
> _tricky_ and deserves a fat comment. So rather than adding flushing work
> item code we should document it properly.
>
> Or am I missing something?
>
But if I read the code correctly, even no one registers a vmpressure event,
vmpressure() is always running and queue the work item.
--
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>
next prev parent reply other threads:[~2013-07-12 9:20 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-10 18:42 workqueue usage in vmpressure Tejun Heo
[not found] ` <20130710184254.GA16979-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-11 8:31 ` Michal Hocko
[not found] ` <20130711083110.GC21667-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-07-11 8:43 ` Li Zefan
[not found] ` <51DE701C.6010800-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-07-11 9:25 ` Michal Hocko
[not found] ` <20130711092542.GD21667-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-07-11 9:28 ` Li Zefan
[not found] ` <51DE7AAF.6070004-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-07-11 9:33 ` [PATCH v2] vmpressure: make sure memcg stays alive until all users are signaled Michal Hocko
2013-07-11 9:33 ` Michal Hocko
2013-07-11 15:44 ` Tejun Heo
[not found] ` <20130711154408.GA9229-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-11 16:22 ` Michal Hocko
2013-07-11 16:22 ` Michal Hocko
2013-07-11 16:32 ` Tejun Heo
2013-07-12 8:40 ` Michal Hocko
[not found] ` <20130712084039.GA13224-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-07-12 9:20 ` Li Zefan [this message]
2013-07-12 9:20 ` Li Zefan
[not found] ` <51DFCA49.4080407-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-07-12 9:29 ` Michal Hocko
2013-07-12 9:29 ` Michal Hocko
[not found] ` <20130712092927.GA15307-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-07-12 9:54 ` Li Zefan
2013-07-12 9:54 ` Li Zefan
2013-07-12 10:37 ` Michal Hocko
[not found] ` <20130712103731.GB15307-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-07-15 3:07 ` Li Zefan
2013-07-15 3:07 ` Li Zefan
2013-07-15 9:20 ` Michal Hocko
[not found] ` <20130715092033.GB26199-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-07-15 9:53 ` Li Zefan
2013-07-15 9:53 ` Li Zefan
2013-07-12 9:24 ` [PATCH 1/3] vmpressure: document why css_get/put is not necessary for work queue based signaling Michal Hocko
2013-07-12 9:24 ` [PATCH 2/3] vmpressure: change vmpressure::sr_lock to spinlock Michal Hocko
[not found] ` <1373621098-15261-1-git-send-email-mhocko-AlSwsSmVLrQ@public.gmane.org>
2013-07-12 9:24 ` [PATCH 3/3] vmpressure: do not check for pending work to prevent from new work Michal Hocko
2013-07-12 9:24 ` Michal Hocko
2013-07-12 18:48 ` [PATCH 1/3] vmpressure: document why css_get/put is not necessary for work queue based signaling Tejun Heo
[not found] ` <20130712184836.GC23680-9pTldWuhBndy/B6EtB590w@public.gmane.org>
2013-07-15 10:27 ` Michal Hocko
2013-07-15 10:27 ` Michal Hocko
2013-07-12 18:34 ` [PATCH v2] vmpressure: make sure memcg stays alive until all users are signaled Tejun Heo
2013-07-12 18:40 ` Tejun Heo
[not found] ` <20130711162215.GM21667-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-07-12 6:03 ` Li Zefan
2013-07-12 6:03 ` Li Zefan
2013-07-15 10:30 ` [PATCH v3 1/3] vmpressure: change vmpressure::sr_lock to spinlock Michal Hocko
2013-07-15 10:30 ` Michal Hocko
2013-07-15 10:30 ` [PATCH v3 2/3] vmpressure: do not check for pending work to prevent from new work Michal Hocko
2013-07-15 10:30 ` [PATCH v3 3/3] vmpressure: Make sure there are no events queued after memcg is offlined Michal Hocko
2013-07-11 8:45 ` workqueue usage in vmpressure Li Zefan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51DFCA49.4080407@huawei.com \
--to=lizefan-hv44wf8li93qt0dzr+alfa@public.gmane.org \
--cc=anton.vorontsov-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \
--cc=kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org \
--cc=kosaki.motohiro-+CUm20s59erQFUHtdCDX3A@public.gmane.org \
--cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \
--cc=mhocko-AlSwsSmVLrQ@public.gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.