From: Tejun Heo <tj@kernel.org>
To: Yifan Zhang <zhangyf@marvell.com>
Cc: Jing Xiang <jxiang@marvell.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"yifan.zhangm@gmail.com" <yifan.zhangm@gmail.com>
Subject: Re: [PATCH] workqueue: fix a workqueue kernel panic issue.
Date: Tue, 23 Sep 2014 10:33:03 -0400 [thread overview]
Message-ID: <20140923143303.GC19208@htj.dyndns.org> (raw)
In-Reply-To: <4737A960563B524DA805CA602BE04B307EEEA98F6B@SC-VEXCH2.marvell.com>
On Mon, Sep 22, 2014 at 03:40:55AM -0700, Yifan Zhang wrote:
> You can tell it is a bug when pwq = get_work_pwq() return NULL, and
> cpu_intensive = pwq->wq->flags use it w/o check.
A bug somewhere else.
> Normally get_work_pwq doesn't return NULL, but we had a bug in code
> which makes INIT_WORK(&work, do_work) is called in multi-thread. In
> some cases, work_struct is re-init just before get_work_pwq is
> called, it makes work_struct->data is invalid and thus causes the
> problem. It is indeed a bug of ourselves, and after fix it there is
> no such issue. But I wonder we still a NULL check before dereference
> pwq here anyway, since get_work_pwq may return NULL in some cases.
Do you realize how timing dependent that particular pattern of
breakage is? If you're doing INIT_WORK() in racy way, there are many
places which can break in workqueue. It's not that different from
random memory corruption. It doesn't make any sense at all to add a
special case code for that in one particular place where this specific
incidence happens to trigger. In general, don't do things like this
anywhere in the kernel.
Thanks.
--
tejun
prev parent reply other threads:[~2014-09-23 14:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1410941884-26034-1-git-send-email-zhangyf@marvell.com>
2014-09-22 3:30 ` [PATCH] workqueue: fix a workqueue kernel panic issue Yifan Zhang
2014-09-22 3:39 ` Tejun Heo
2014-09-22 10:40 ` Yifan Zhang
2014-09-23 14:33 ` Tejun Heo [this message]
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=20140923143303.GC19208@htj.dyndns.org \
--to=tj@kernel.org \
--cc=jxiang@marvell.com \
--cc=linux-kernel@vger.kernel.org \
--cc=yifan.zhangm@gmail.com \
--cc=zhangyf@marvell.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).