linux-kernel - Re: [RFC PATCH 1/4] workqueue: support pausing ordered workqueues

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, 10 May 2023 20:40:33 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Tejun Heo <tj@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
        Lai Jiangshan <jiangshanlai@...il.com>
Subject: Re: [RFC PATCH 1/4] workqueue: support pausing ordered workqueues

Hi,

> > -	/* for @wq->saved_max_active */
> > +	/* for @wq->saved_max_active and @wq->flags */
> >  	lockdep_assert_held(&wq->mutex);
> >  
> > +	if (wq->flags & __WQ_PAUSED)
> > +		new_max_active = 0;
> > +	else
> > +		new_max_active = wq->saved_max_active;
> 
> Nothing is using new_max_active and I think we can probably combine this
> with the freezing test.

Err, yikes. Of course this was meant to be used in the remainder of the
code, oops.

Regarding the freezing test, yeah, maybe. It seemed harder to follow,
but I'll take another look.

> > +void __workqueue_pause_resume(struct workqueue_struct *wq, bool pause)
> > +{
> > +	struct pool_workqueue *pwq;
> > +
> > +	mutex_lock(&wq->mutex);
> > +	if (pause)
> > +		wq->flags |= __WQ_PAUSED;
> > +	else
> > +		wq->flags &= ~__WQ_PAUSED;
> > +
> > +	for_each_pwq(pwq, wq)
> > +		pwq_adjust_max_active(pwq);
> > +	mutex_unlock(&wq->mutex);
> > +
> > +	if (pause)
> > +		flush_workqueue(wq);
> > +}
> > +EXPORT_SYMBOL_GPL(__workqueue_pause_resume);
> 
> I'd just make pause and resume separate functions. The sharing ratio doesn't
> seem that high.
> 

Yeah, I wasn't really sure about that either. I keep thinking the
EXPORT_SYMBOL_GPL() itself is really big, but I'm not even sure about
that, and it's probably not a great reason anyway.

johannes