Linux ATA/IDE development
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Marco Crivellari <marco.crivellari@suse.com>
Cc: linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org,
	Tejun Heo <tj@kernel.org>, Lai Jiangshan <jiangshanlai@gmail.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Michal Hocko <mhocko@suse.com>,
	Damien Le Moal <dlemoal@kernel.org>
Subject: Re: [RFC PATCH] ata: libata-scsi: Move long delayed work on system_dfl_long_wq
Date: Mon, 11 May 2026 18:39:37 +0200	[thread overview]
Message-ID: <agIGSYxbK3FX2mXK@ryzen> (raw)
In-Reply-To: <CAAofZF7NhFUHKhp-HdvSx2doFRc4XFERCL22V+BZXrQVjmxugw@mail.gmail.com>

On Mon, May 11, 2026 at 02:54:26PM +0200, Marco Crivellari wrote:
> On Mon, May 11, 2026 at 2:48 PM Niklas Cassel <cassel@kernel.org> wrote:
> > [...]
> > Looks good to me.
> >
> > Any particular reason that you sent this as an RFC?
> >
> > I can see similar patches queued up in linux-next already.
> 
> I just wanted to be sure I didn't miss any other reason for being
> per-cpu, and in case
> receive comments on it.

Hmm... I can see that:
drivers/ata/libata-eh.c:ata_scsi_port_error_handler()
does:

schedule_delayed_work(&ap->hotplug_task, 0);

schedule_delayed_work() does:
queue_delayed_work(system_percpu_wq, dwork, delay);

So this will schedule the work on a per-cpu workqueue.


It seems that we are already queueing the same work (&ap->hotplug_task)
on different workqueues, so I guess that is fine.

Right now, both workqueues are per-cpu. Is it fine to change one of them
to be not be bound to a specific CPU?

From looking at the work, ata_scsi_hotplug(), I can't think of a reason
why this would have to run on the same CPU as the CPU that queued the
work.



From looking at workqueue.h:

 * system_dfl_wq is unbound workqueue.  Workers are not bound to
 * any specific CPU, not concurrency managed, and all queued works are
 * executed immediately as long as max_active limit is not reached and
 * resources are available.

 [...]

 * system_dfl_long_wq is similar to system_dfl_wq but it may host long running
 * works.

"not concurrency managed"

That sounds like a big change, since the per-cpu workqueues do seem to be
concurrency managed (unlike the _dfl_ ones).

However, considering that the work (&ap->hotplug_task / ata_scsi_hotplug())
does:
mutex_lock(&ap->scsi_scan_mutex);

I also don't see a problem with the workqueue not being concurrency managed,
since the work is taking a mutex anyway.



If anyone sees a problem, please say something, otherwise intend to queue
this up in a few days.


Kind regards,
Niklas

  reply	other threads:[~2026-05-11 16:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-30  9:29 [RFC PATCH] ata: libata-scsi: Move long delayed work on system_dfl_long_wq Marco Crivellari
2026-05-11 12:48 ` Niklas Cassel
2026-05-11 12:54   ` Marco Crivellari
2026-05-11 16:39     ` Niklas Cassel [this message]
2026-05-12 12:31       ` Frederic Weisbecker
2026-05-13  7:43 ` Niklas Cassel
2026-05-13  8:14   ` Marco Crivellari

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=agIGSYxbK3FX2mXK@ryzen \
    --to=cassel@kernel.org \
    --cc=bigeasy@linutronix.de \
    --cc=dlemoal@kernel.org \
    --cc=frederic@kernel.org \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marco.crivellari@suse.com \
    --cc=mhocko@suse.com \
    --cc=tj@kernel.org \
    /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