All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@suse.de>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: William Lee Irwin III <wli@holomorphy.com>,
	Andrew Morton <akpm@osdl.org>,
	marcelo.tosatti@cyclades.com, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] oom killer (Core)
Date: Fri, 24 Dec 2004 02:18:33 +0100	[thread overview]
Message-ID: <20041224011833.GE2143@dualathlon.random> (raw)
In-Reply-To: <1102697553.3306.91.camel@tglx.tec.linutronix.de>

On Fri, Dec 10, 2004 at 05:52:33PM +0100, Thomas Gleixner wrote:
> +			if ((p->flags & PF_MEMDIE) ||
> +			    ((p->flags & PF_EXITING) && !(p->flags & PF_DEAD)))

this had to be:

			if (((p->flags & PF_MEMDIE) || (p->flags & PF_EXITING)) &&
			    !(p->flags & PF_DEAD))

I didn't take zombies into account. A task may be in memdie state and zombie at
the same time. We must not wait a PF_MEMDIE to go away completely, we must wait
only until PF_DEAD is not set. After PF_DEAD is set we know all ram we
were waiting for is already in the free list.

I noticed some deadlocks during an oom-torture-test before applying the stuff
into the suse kernel. The above change fixed all my deadlocks. Everything else
was working fine already.

Actually before finding the above bug I fixed PF_MEMDIE too and I converted it
to p->memdie, an unsigned char. All archs should support 1 byte granularity
for per-process atomic instructions, it's near used_math that I also converted
to a char to signal it cannot be a bitflag sharing the same char with globals.

Struct layout looks like this.

	/*
	 * All archs should support atomic ops with
	 * 1 byte granularity.
	 */
	unsigned char memdie;
	/*
	 * Must be changed atomically so it shouldn't be
	 * be a shareable bitflag.
	 */
	unsigned char used_math;
	/*
	 * OOM kill score adjustment (bit shift).
	 * Cannot live together with used_math since
	 * used_math and oomkilladj can be changed at the
	 * same time, so they would race if they're in the
	 * same atomic block.
	 */
	short oomkilladj;

As for the |= PF_MEMALLOC in oom_kill_task that was a gratuitous smp breakage,
I didn't need to do anything in PF_MEMALLOC since alloc_pages checks _both_
PF_MEMALLOC and p->memdie already.

I also added a !chosen in the below code to make that logic more self
contained and less dependent on the badness implementation (should never
be necessary though):

			if (points > maxpoints || !chosen) {
				chosen = p;
				maxpoints = points;
			}

I can port the final patch (including fixage of PF_MEMDIE races) to 2.6.10-rc
after I finished.

  parent reply	other threads:[~2004-12-24  1:19 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-12-01  9:49 [PATCH] oom killer (Core) tglx
2004-12-01 21:16 ` Andrea Arcangeli
2004-12-01 22:06   ` Thomas Gleixner
2004-12-01 22:33     ` Andrea Arcangeli
2004-12-02  3:36     ` Andrea Arcangeli
2004-12-02 11:09       ` Thomas Gleixner
2004-12-02 13:48         ` Thomas Gleixner
2004-12-02 16:47           ` Andrea Arcangeli
2004-12-02 16:55             ` Andrew Morton
2004-12-02 11:18               ` Marcelo Tosatti
2004-12-02 17:17               ` Thomas Gleixner
2004-12-02 17:27                 ` Andrew Morton
2004-12-02 18:08               ` Andrea Arcangeli
2004-12-02 18:29                 ` Andrew Morton
2004-12-02 19:01                   ` Thomas Gleixner
2004-12-02 18:55                 ` Thomas Gleixner
2004-12-02 19:07                   ` Andrew Morton
2004-12-02 19:08                     ` Thomas Gleixner
2004-12-02 19:22                       ` Andrew Morton
2004-12-02 19:24                         ` Thomas Gleixner
2004-12-02 20:11                           ` Andre Tomt
2004-12-03 22:45                             ` Thomas Gleixner
2004-12-02 23:47                           ` Andrea Arcangeli
2004-12-03 14:41                           ` Helge Hafting
2004-12-03 21:20                             ` Thomas Gleixner
2004-12-05 21:14                               ` Helge Hafting
2004-12-02 23:35                   ` Andrea Arcangeli
2004-12-03  2:28                     ` Andrea Arcangeli
2004-12-03 22:37                       ` Thomas Gleixner
2004-12-03 22:51                         ` Thomas Gleixner
2004-12-03 23:08                           ` Andrea Arcangeli
2004-12-10 16:36                       ` William Lee Irwin III
2004-12-10 17:35                         ` Andrea Arcangeli
2004-12-10 17:43                           ` William Lee Irwin III
2004-12-10 17:55                             ` Andrea Arcangeli
2004-12-10 18:00                               ` William Lee Irwin III
2004-12-10 18:15                                 ` Andrea Arcangeli
2004-12-10 18:19                                   ` William Lee Irwin III
2004-12-10 19:05                                     ` Andrea Arcangeli
2004-12-10 16:51                       ` William Lee Irwin III
2004-12-03 21:10                     ` Thomas Gleixner
2004-12-03 22:21                       ` Andrea Arcangeli
2004-12-05  2:52 ` William Lee Irwin III
2004-12-05 13:38   ` Thomas Gleixner
2004-12-05 15:22     ` Andrea Arcangeli
2004-12-10 16:32 ` William Lee Irwin III
2004-12-10 16:52   ` Thomas Gleixner
2004-12-10 17:43     ` William Lee Irwin III
2004-12-10 17:47     ` William Lee Irwin III
2004-12-10 17:49     ` Andrea Arcangeli
2004-12-10 17:57       ` William Lee Irwin III
2004-12-12  0:12         ` William Lee Irwin III
2004-12-24  1:18     ` Andrea Arcangeli [this message]
  -- strict thread matches above, loose matches on Subject: below --
2004-12-01 10:21 tvrtko.ursulin
2004-12-04  7:00 Voluspa
2004-12-04  8:08 ` Andrea Arcangeli
2004-12-04 12:42 Voluspa
2004-12-04 16:43 ` Andrea Arcangeli
2004-12-04 18:33   ` Thomas Gleixner
2004-12-04 21:02     ` Thomas Gleixner
2004-12-05  0:27       ` Andrea Arcangeli
2004-12-05 14:55         ` Thomas Gleixner
2004-12-05 15:34           ` Andrea Arcangeli
2004-12-05 16:29             ` Thomas Gleixner
2004-12-05  2:22 Voluspa
2004-12-05  8:32 Voluspa

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=20041224011833.GE2143@dualathlon.random \
    --to=andrea@suse.de \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo.tosatti@cyclades.com \
    --cc=tglx@linutronix.de \
    --cc=wli@holomorphy.com \
    /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.