linux-kernel - Re: [PATCH 00/10] workqueue: restructure flush_workqueue() and start all flusher at the same time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 26 Sep 2012 09:49:58 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 00/10] workqueue: restructure flush_workqueue() and
 start all flusher at the same time

Hello, Lai.

On Wed, Sep 26, 2012 at 12:48:59PM +0800, Lai Jiangshan wrote:
> > Hmmm... so, that's a lot simpler.  flush_workqueue() isn't a super-hot
> > code path and I don't think grabbing mutex twice is too big a deal.  I
> > haven't actually reviewed the code but if it can be much simpler and
> > thus easier to understand and verify, I might go for that.
> 
> I updated it. it is attached, it forces flush_workqueue() to grab
> mutex twice(no other forcing).  overflow queue is implemented in a
> different way. This new algorithm may become our choice likely,
> please review this one.

Will do shortly.

> I did not know this history, thank you.
> 
> But the number of colors is not essential.
> "Does the algorithm chain flushers" is essential.
> 
> If we can have multiple flushers for each color. It is not chained.
> If we have only one flusher for one color. It is chained. Even we
> have multiple color, it is still partially chained(image we have
> very high frequent flush_workqueue()).

If you have very few colors, you can end up merging flushes of a lot
of work items which in turn delays the next flush and thus merging
more of them.  This was what Linus was worried about.

> The initial implementation of flush_workqueue() is "chained" algorithm.

I don't know what you mean by "chained" here.  The current mainline
implementation has enough colors for most use cases and don't assign a
color to single work item.  It's specifically designed to avoid
chained latencies.

> The initial implementation of SRCU is also "chained" algorithm.
> but the current SRCU which was implemented by me is not "chained"
> (I don't propose to use SRCU for flush_workqueue(), I just discuss it)

So, you lost me.  The current implementation doesn't have a problem on
that front.

> The simple version of flush_workqueue() which I sent yesterday is "chained",
> because it forces overflow flushers wait for free color and forces only one
> flusher for one color.
>
> Since "not chaining" is important/essential. I sent a new draft implement today.
> it uses multiple queues, one for each color(like SRCU).
> this version is also simple, it remove 90 LOC.

I'll review your patch but the current implementation is enough on
that regard.  I was trying to advise against going for two-color
scheme.

Thanks.

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