On Thu 18-10-12 21:51:57, Sha Zhengju wrote:On 10/18/2012 07:56 PM, Michal Hocko wrote:Hmm filemap_fault locks the page after the read ahead is triggeredOn Wed 17-10-12 01:14:48, Sha Zhengju wrote:No, it has nothing with the cow page. By checking stack of the process AOn Tuesday, October 16, 2012, Michal Hocko<mhocko@xxxxxxx> wrote:[...]Hmm, this is strange. How can you get down that road with the page lockCould you be more specific about the motivation for this patch? Is itIn our environment(rhel6), we encounter a memcg oom 'deadlock'
"let's be consistent with the global oom" or you have a real use case
for this knob.
problem. Simply speaking, suppose process A is selected to be killed
by memcg oom killer, but A is uninterruptible sleeping on a page
lock. What's worse, the exact page lock is holding by another memcg
process B which is trapped in mem_croup_oom_lock(proves to be a
livelock).
held? Is it possible this is related to the issue fixed by: 1d65f86d
(mm: preallocate page before lock_page() at filemap COW)?
selected to be killed(uninterruptible sleeping), it was stuck at:
__do_fault->filemap_fault->__lock_page_or_retry->wait_on_page_bit--(D
state).
The person B holding the exactly page lock is on the following path:
__do_fault->filemap_fault->__do_page_cache_readahead->..->mpage_readpages
->add_to_page_cache_locked ---->(in memcg oom and cannot exit)
already so it doesn't call mpage_readpages with any page locked - the
add_to_page_cache_lru is called without any page locked.
This is at least the current code. It might be different in rhel6 but
calling memcg charging with a page lock is definitely a bug.
In mpage_readpages, B tends to read a dozen of pages in: for each of79dfdaccd1d5 you mean, right? That one just helps when there are too
page will do
locking, charging, and then send out a big bio. And A is waiting for
one of the pages
and stuck.
As I said, 37b23e05 has made pagefault killable by changing
uninterruptible sleeping to killable sleeping. So A can be woke up to
exit successfully and free the memory which can in turn help B pass
memcg charging period.
(By the way, it seems commit 37b23e05 and 7d9fdac need to be
many tasks trashing oom killer so it is not related to what you are
trying to achieve. Besides that make sure you take 23751be0 if you
take it.
backported to --stable tree to deliver RHEL users. ;-) )I am not sure the first one qualifies the stable tree inclusion as it is
a feature.
I am not sure about "no obvious harm" part. The policy could beIn view of consistent with global oom and performance benefit, I suggestThen A can not exit successfully to free the memory and both of themAs I said, I am not against this but I really want to see a valid use
can not moving on.
Indeed, we should dig into these locks to find the solution and
in fact the 37b23e05 (x86, mm: make pagefault killable) and
7d9fdac(Memcg: make oom_lock 0 and 1 based other than counter) have
already solved the problem, but if oom_killing_allocating_task is
memcg aware, enabling this suicide oom behavior will be a simpler
workaround. What's more, enabling the sysctl can avoid other potential
oom problems to some extent.
case first. So far I haven't seen any because what you mention above is
a clear bug which should be fixed. I can imagine the huge number of
tasks in the group could be a problem as well but I would like to see
what are those problems first.
we may as well open it in memcg oom as there's no obvious harm.
different in different groups e.g. and the global knob could be really
misleading. But the question is. Is it worth having this per group? To
be honest, I do not like the global knob either and I am not entirely
keen on spreading it out into memcg unless there is a real use case for
it.
As refer to the bug I mentioned, obviously the key solution is the above twoWe shouldn't add something like that just to workaround obvious bugs.
patchset, but considing other *potential* memcg oom bugs, the sysctl may
be a role of temporary workaround to some extent... but it's just a
workaround.