All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <dahi@linux.vnet.ibm.com>
To: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org, borntraeger@de.ibm.com,
	rafael.j.wysocki@intel.com, peterz@infradead.org,
	oleg@redhat.com, bp@suse.de, jkosina@suse.cz
Subject: Re: [PATCH v2] CPU hotplug: active_writer not woken up in some cases - deadlock
Date: Tue, 9 Dec 2014 11:11:01 +0100	[thread overview]
Message-ID: <20141209111101.201e3544@thinkpad-w530> (raw)
In-Reply-To: <20141209091447.GD4362@osiris>

> > Therefore we have to move the condition check inside the 
> >   __set_current_state(TASK_UNINTERRUPTIBLE) -> schedule();
> > section to not miss any wake ups when the condition is satisfied.
> > 
> > So wake_up_process() will either see TASK_RUNNING and do nothing or see
> > TASK_UNINTERRUPTIBLE and set it to TASK_RUNNING, so schedule() will in
> > fact be woken up again.
> 
> Or the third alternative would be that 'active_writer' which was running
> on CPU2 already terminated and wake_up_process() has a non-NULL pointer to
> task_struct which is already dead.
> Or is there anything that prevents this use-after-free race?

Hmmm ... I think that is also a valid scenario.
That would mean we need soemthing like this:

 void put_online_cpus(void)
 {
+ struct task_struct *awr;
+
        if (cpu_hotplug.active_writer == current)
                return;
        if (!mutex_trylock(&cpu_hotplug.lock)) {
+         awr = ACCESS_ONCE(cpu_hotplug.active_writer);
+         if (unlikely(awr))
+                 get_task_struct(awr);
+         /* inc after get_task_struct(), so the writer can't get NULL */
                atomic_inc(&cpu_hotplug.puts_pending);
+         /* we might be the last one */
+         if (unlikely(awr)) {
+                 wake_up_process(awr);
+                 put_task_struct(awr);
+         }
                cpuhp_lock_release();
                return;
        }


Thanks!

David


  reply	other threads:[~2014-12-09 10:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-08 20:21 [PATCH v2] CPU hotplug: active_writer not woken up in some cases - deadlock David Hildenbrand
2014-12-08 21:22 ` Paul E. McKenney
2014-12-09  7:59   ` David Hildenbrand
2014-12-09  9:14     ` Heiko Carstens
2014-12-09 10:11       ` David Hildenbrand [this message]
2014-12-09 10:21         ` Heiko Carstens
2014-12-09 11:04           ` David Hildenbrand
2014-12-09 11:35           ` David Hildenbrand

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=20141209111101.201e3544@thinkpad-w530 \
    --to=dahi@linux.vnet.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=bp@suse.de \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.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.