* [PATCH v2 (REPOST)] rapidio/tsi721: Replace flush_scheduled_work() with flush_work().
@ 2022-07-28 1:02 Tetsuo Handa
2022-08-15 0:14 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Tetsuo Handa @ 2022-07-28 1:02 UTC (permalink / raw)
To: Matt Porter, Alexandre Bounine; +Cc: LKML, Andrew Morton
Since "struct tsi721_device" is per a device struct, I assume that
tsi721_remove() needs to wait for only two works associated with that
device. Therefore, wait for only these works using flush_work().
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Changes in v2:
Use flush_work() instead of introducing a dedicated WQ.
Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue()
using a macro") for background.
drivers/rapidio/devices/tsi721.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/rapidio/devices/tsi721.c b/drivers/rapidio/devices/tsi721.c
index b3134744fb55..0a42d6a2af24 100644
--- a/drivers/rapidio/devices/tsi721.c
+++ b/drivers/rapidio/devices/tsi721.c
@@ -2941,7 +2941,8 @@ static void tsi721_remove(struct pci_dev *pdev)
tsi721_disable_ints(priv);
tsi721_free_irq(priv);
- flush_scheduled_work();
+ flush_work(&priv->idb_work);
+ flush_work(&priv->pw_work);
rio_unregister_mport(&priv->mport);
tsi721_unregister_dma(priv);
--
2.18.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 (REPOST)] rapidio/tsi721: Replace flush_scheduled_work() with flush_work().
2022-07-28 1:02 [PATCH v2 (REPOST)] rapidio/tsi721: Replace flush_scheduled_work() with flush_work() Tetsuo Handa
@ 2022-08-15 0:14 ` Andrew Morton
2022-08-15 9:47 ` Tetsuo Handa
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2022-08-15 0:14 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: Matt Porter, Alexandre Bounine, LKML
On Thu, 28 Jul 2022 10:02:13 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> Since "struct tsi721_device" is per a device struct, I assume that
> tsi721_remove() needs to wait for only two works associated with that
> device. Therefore, wait for only these works using flush_work().
The changelog provides no reason for making this change. Correctness?
Efficiency?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 (REPOST)] rapidio/tsi721: Replace flush_scheduled_work() with flush_work().
2022-08-15 0:14 ` Andrew Morton
@ 2022-08-15 9:47 ` Tetsuo Handa
2022-09-16 14:21 ` Tetsuo Handa
0 siblings, 1 reply; 4+ messages in thread
From: Tetsuo Handa @ 2022-08-15 9:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Matt Porter, Alexandre Bounine, LKML
On 2022/08/15 9:14, Andrew Morton wrote:
> On Thu, 28 Jul 2022 10:02:13 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>
>> Since "struct tsi721_device" is per a device struct, I assume that
>> tsi721_remove() needs to wait for only two works associated with that
>> device. Therefore, wait for only these works using flush_work().
>
> The changelog provides no reason for making this change. Correctness?
> Efficiency?
For reducing locking dependency chains that might lead to deadlock.
The reason was explained in commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue()
using a macro"). For example, after the fact it turned out that a blind conversion patch
fixed a deadlock problem at https://syzkaller.appspot.com/bug?extid=2d6ac90723742279e101
which was hidden due to use of lockdep_set_novalidate_class() call.
Many of flush_scheduled_work() callers have been removed by v6.0-rc1, but not all
patches were verbose (because the reason was already explained).
a1124c84d467 power: supply: ab8500: Remove flush_scheduled_work() call.
162b05524ed3 rtc: Replace flush_scheduled_work() with flush_work().
31a1e4a5c104 platform/surface: avoid flush_scheduled_work() usage
90c3ca3f247d scsi: mpt3sas: Remove flush_scheduled_work() call
62ebaf2f9261 ath6kl: avoid flush_scheduled_work() usage
76faa32077b0 iio: light: tsl2563: Replace flush_scheduled_work() with cancel_delayed_work_sync().
4bbdc208a5ff staging: olpc_dcon: Replace flush_scheduled_work() with flush_work().
c4f135d64382 workqueue: Wrap flush_workqueue() using a macro
9cf62d91e4b7 RDMA/mlx4: Avoid flush_scheduled_work() usage
549f39a58acf IB/isert: Avoid flush_scheduled_work() usage
1b3ce51dde36 Input: psmouse-smbus - avoid flush_scheduled_work() usage
0a2f4b5785ca crypto: atmel - Avoid flush_scheduled_work() usage
eeff214dbfcb wfx: avoid flush_workqueue(system_highpri_wq) usage
0b8d7622ab18 aoe: Avoid flush_scheduled_work() usage
cc271ab86606 wwan_hwsim: Avoid flush_scheduled_work() usage
ff815a89398d RDMA/core: Avoid flush_workqueue(system_unbound_wq) usage
There are 7 callers remaining as of v6.0-rc1, and I'm pinging them for heads up.
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
drivers/gpu/drm/i915/display/intel_display.c
drivers/gpu/drm/i915/gt/selftest_execlists.c
drivers/md/dm.c
drivers/message/fusion/mptscsih.c
drivers/rapidio/devices/tsi721.c
drivers/scsi/qla2xxx/qla_target.c
If you want to update patch description, you can insert the following.
----------------------------------------
Like commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a
macro") says, flush_scheduled_work() is dangerous and will be forbidden.
We are on the way for removing all flush_scheduled_work() callers from
the kernel, and this patch is for removing flush_scheduled_work() call
from tsi721 driver.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 (REPOST)] rapidio/tsi721: Replace flush_scheduled_work() with flush_work().
2022-08-15 9:47 ` Tetsuo Handa
@ 2022-09-16 14:21 ` Tetsuo Handa
0 siblings, 0 replies; 4+ messages in thread
From: Tetsuo Handa @ 2022-09-16 14:21 UTC (permalink / raw)
To: Matt Porter, Alexandre Bounine; +Cc: LKML, Andrew Morton
Matt and Alexandre, what can I do on this patch?
On 2022/08/15 18:47, Tetsuo Handa wrote:
> On 2022/08/15 9:14, Andrew Morton wrote:
>> On Thu, 28 Jul 2022 10:02:13 +0900 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>>
>>> Since "struct tsi721_device" is per a device struct, I assume that
>>> tsi721_remove() needs to wait for only two works associated with that
>>> device. Therefore, wait for only these works using flush_work().
>>
>> The changelog provides no reason for making this change. Correctness?
>> Efficiency?
>
> For reducing locking dependency chains that might lead to deadlock.
>
> The reason was explained in commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue()
> using a macro"). For example, after the fact it turned out that a blind conversion patch
> fixed a deadlock problem at https://syzkaller.appspot.com/bug?extid=2d6ac90723742279e101
> which was hidden due to use of lockdep_set_novalidate_class() call.
>
> Many of flush_scheduled_work() callers have been removed by v6.0-rc1, but not all
> patches were verbose (because the reason was already explained).
>
> a1124c84d467 power: supply: ab8500: Remove flush_scheduled_work() call.
> 162b05524ed3 rtc: Replace flush_scheduled_work() with flush_work().
> 31a1e4a5c104 platform/surface: avoid flush_scheduled_work() usage
> 90c3ca3f247d scsi: mpt3sas: Remove flush_scheduled_work() call
> 62ebaf2f9261 ath6kl: avoid flush_scheduled_work() usage
> 76faa32077b0 iio: light: tsl2563: Replace flush_scheduled_work() with cancel_delayed_work_sync().
> 4bbdc208a5ff staging: olpc_dcon: Replace flush_scheduled_work() with flush_work().
> c4f135d64382 workqueue: Wrap flush_workqueue() using a macro
> 9cf62d91e4b7 RDMA/mlx4: Avoid flush_scheduled_work() usage
> 549f39a58acf IB/isert: Avoid flush_scheduled_work() usage
> 1b3ce51dde36 Input: psmouse-smbus - avoid flush_scheduled_work() usage
> 0a2f4b5785ca crypto: atmel - Avoid flush_scheduled_work() usage
> eeff214dbfcb wfx: avoid flush_workqueue(system_highpri_wq) usage
> 0b8d7622ab18 aoe: Avoid flush_scheduled_work() usage
> cc271ab86606 wwan_hwsim: Avoid flush_scheduled_work() usage
> ff815a89398d RDMA/core: Avoid flush_workqueue(system_unbound_wq) usage
>
> There are 7 callers remaining as of v6.0-rc1, and I'm pinging them for heads up.
>
> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> drivers/gpu/drm/i915/display/intel_display.c
> drivers/gpu/drm/i915/gt/selftest_execlists.c
> drivers/md/dm.c
> drivers/message/fusion/mptscsih.c
> drivers/rapidio/devices/tsi721.c
> drivers/scsi/qla2xxx/qla_target.c
>
> If you want to update patch description, you can insert the following.
>
> ----------------------------------------
>
> Like commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a
> macro") says, flush_scheduled_work() is dangerous and will be forbidden.
> We are on the way for removing all flush_scheduled_work() callers from
> the kernel, and this patch is for removing flush_scheduled_work() call
> from tsi721 driver.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-16 14:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-28 1:02 [PATCH v2 (REPOST)] rapidio/tsi721: Replace flush_scheduled_work() with flush_work() Tetsuo Handa
2022-08-15 0:14 ` Andrew Morton
2022-08-15 9:47 ` Tetsuo Handa
2022-09-16 14:21 ` Tetsuo Handa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.