All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Jackson <pj@sgi.com>
To: "Paul Menage" <menage@google.com>
Cc: vatsa@in.ibm.com, akpm@osdl.org, sekharan@us.ibm.com, dev@sw.ru,
	xemul@sw.ru, ckrm-tech@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, mbligh@google.com,
	winget@google.com, rohitseth@google.com, devel@openvz.org,
	mingo@elte.hu, nickpiggin@yahoo.com.au, dipankar@in.ibm.com,
	balbir@in.ibm.com
Subject: Re: [Patch 1/3] Miscellaneous container fixes
Date: Fri, 1 Dec 2006 12:31:34 -0800	[thread overview]
Message-ID: <20061201123134.106da1c2.pj@sgi.com> (raw)
In-Reply-To: <6599ad830612010925w17f56643n8c92f179ea28b828@mail.gmail.com>

Paul M wrote:
> Ah - this may be the lockup that PaulJ hit.

Yes - looks like this fixes it.  Thanks, Srivatsa.

And with that fix, it becomes obvious how to reproduce this problem:

	mount -t cpuset cpuset /dev/cpuset	# if not already mounted
	cd /dev/cpuset
	mkdir foo
	echo 1 > foo/cpu_exclusive
	rmdir foo				# hangs ...

However ...

Read the comment in kernel/cpuset.c for the routine cpuset_destroy().
It explains that update_flag() is called where it is (turning off
the cpu_exclusive flag, if it was set), to avoid the calling sequence:

  cpuset_destroy->update_flag->update_cpu_domains->lock_cpu_hotplug

while holding the callback_mutex, as that could ABBA deadlock with the
CPU hotplug code.

But with this container based rewrite of cpusets, it now seems that
cpuset_destroy -is- called holding the callback_mutex (though I don't
see any mention of that in the cpuset_destroy comment ;), so it would
seem that we once again are at risk for this ABBA deadlock.

I also notice that the comment for container_lock() in the file
kernel/container.c only mentions its use in the oom code.  That is
no longer the only, or even primary, user of this lock routine.
The kernel/cpuset.c code uses it frequently (without comment ;),
and I wouldn't be surprised to see other future controllers calling
container_lock() as well.

Looks like its time to update those comments, and think about what
was written there before, as that might catch a bug or two, such as
the one Srivatsa just fixed for us.

Most of those long locking comments in kernel/cpuset.c are there
for a reason - recording the results of a lesson learned in the
school of hard knocks.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

  reply	other threads:[~2006-12-01 20:33 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-23 12:08 [PATCH 0/7] Generic Process Containers (+ ResGroups/BeanCounters) menage
2006-11-23 12:08 ` [PATCH 1/7] Generic container system abstracted from cpusets code menage
2006-11-23 12:08 ` [PATCH 2/7] Cpusets hooked into containers menage
2006-12-01 16:46   ` [Patch 1/3] Miscellaneous container fixes Srivatsa Vaddagiri
2006-12-01 16:51     ` [Patch 2/3] cpu controller (v3) - based on RTLIMIT_RT_CPU patch Srivatsa Vaddagiri
2006-12-01 16:53     ` [Patch 3/3] cpuset interface Srivatsa Vaddagiri
2006-12-01 17:25     ` [Patch 1/3] Miscellaneous container fixes Paul Menage
2006-12-01 20:31       ` Paul Jackson [this message]
2006-12-05 12:04         ` Paul Menage
2006-11-23 12:08 ` [PATCH 3/7] Add generic multi-subsystem API to containers menage
2006-11-23 12:08 ` [PATCH 4/7] Simple CPU accounting container subsystem menage
2006-11-23 12:08 ` [PATCH 5/7] Resource Groups over generic containers menage
2006-11-23 12:08 ` [PATCH 7/7] BeanCounters over generic process containers menage
2006-11-30  7:32 ` [PATCH 0/7] Generic Process Containers (+ ResGroups/BeanCounters) Paul Jackson
2006-11-30  8:01   ` Paul Menage
2006-11-30  8:51   ` Paul Menage

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=20061201123134.106da1c2.pj@sgi.com \
    --to=pj@sgi.com \
    --cc=akpm@osdl.org \
    --cc=balbir@in.ibm.com \
    --cc=ckrm-tech@lists.sourceforge.net \
    --cc=dev@sw.ru \
    --cc=devel@openvz.org \
    --cc=dipankar@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbligh@google.com \
    --cc=menage@google.com \
    --cc=mingo@elte.hu \
    --cc=nickpiggin@yahoo.com.au \
    --cc=rohitseth@google.com \
    --cc=sekharan@us.ibm.com \
    --cc=vatsa@in.ibm.com \
    --cc=winget@google.com \
    --cc=xemul@sw.ru \
    /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.