Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <920f4c3a5dd5183628ad6c1196b5cc689c1ebeeb.camel@sipsolutions.net>
Subject: Re: wireless locking simplifications
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Tejun Heo <tj@kernel.org>
Cc:     linux-wireless@vger.kernel.org,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Benjamin Berg <benjamin@sipsolutions.net>
Date:   Mon, 08 May 2023 18:06:35 +0200
In-Reply-To: <ZFV6fTH_7umEa5H0@slm.duckdns.org>
References: <f3471853cd7063a4bd2d783caa14706ee9115748.camel@sipsolutions.net>
         <ZFV6fTH_7umEa5H0@slm.duckdns.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-05-05 at 11:51 -1000, Tejun Heo wrote:
> Hello, Johannes.
>=20
> On Fri, May 05, 2023 at 11:05:45PM +0200, Johannes Berg wrote:
> ...
> > The implementation of (2) is a bit ... awkward, @Tejun, @Lai, there's n=
o
> > way to "pause" an ordered workqueue, is there? I came up with the below
> > patch, but it's a bit ugly and requires a lot of context switches. Just
> > flushing isn't enough since then some work might start and hang on
> > acquiring the lock.
>=20
> There isn't currently but workqueue already does something similar for
> freezing by temporarily setting max_active to zero,
>=20

Ah, nice. I had thought about it but a lot of places didn't like 0 so
wasn't sure whether or not it was allowed :)

> [snip]

> and then do set_max_active(wq, 0) followed by flush_workqueue(), it shoul=
d
> be paused until max_active is restored to 1. It probably would be better =
to
> add a separate pause / resume interface which sets a per-wq flag which is
> read by pwq_adjust_max_active() tho. Anyways, it's not difficult to
> implement at all.

Ok cool, I'll take a look at it at some point. I think it would greatly
simplify the locking etc. in wifi because then we don't need to worry
about the cancel_work_sync() or flush_work() or flush_workqueue()
deadlocks.

Thanks!

johannes