Date: Tue, 29 Jul 2008 17:44:57 +0400
From: Oleg Nesterov <oleg@tv-sign.ru>
To: Dmitry Adamushko <dmitry.adamushko@gmail.com>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>
Subject: Re: [patch, minor] workqueue: consistently use 'err' in __create_workqueue_key()
Message-ID: <20080729134456.GA355@tv-sign.ru>
References: <1217277694.20627.9.camel@earth> <20080729110250.GA177@tv-sign.ru> <b647ffbd0807290458n23dbe79et45cb27273e4fc2ae@mail.gmail.com> <20080729125201.GC177@tv-sign.ru>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20080729125201.GC177@tv-sign.ru>
User-Agent: Mutt/1.5.11
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1271
Lines: 34

On 07/29, Oleg Nesterov wrote:
>
> On 07/29, Dmitry Adamushko wrote:
> >
> > And I'd say this behavior (of having a partially-created object
> > visible to the outside world) is not that robust. e.g. the
> > aforementioned race would be eliminated if we place a wq on the global
> > list only when it's been successfully initialized.
>
> Yes, we can change __create_workqueue_key() to check err == 0 before
> list_add(),

Well no, we can't do even this.

Then we have another race with cpu-hotplug. Suppose we have CPUs 0, 1, 2.
create_workqueue() fails to create cwq->thread for CPU 2 and calls
destroy_workqueue(). Before it takes the cpu_add_remove_lock, _cpu_down()
removes CPU 1 from cpu_populated_map, but since we didn't add this wq
on the global list, cwq[1]->thread remains alive.

destroy_workqueue() takes cpu_add_remove_lock, and calls
cleanup_workqueue_thread() for CPUs 0 and 2. cwq[1]->thread is lost.


Damn. I had this in mind when I wrote the code, but forgot. We need
comments, I'll send the patch.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/