| Message ID | 20220615234219.178186-1-fenghua.yu@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] dmaengine: idxd: force wq context cleanup on device disable path | expand |
On Wed, Jun 15, 2022 at 04:42:19PM -0700, Fenghua Yu wrote: > From: Dave Jiang <dave.jiang@intel.com> > > Testing shown that when a wq mode is setup to be dedicated and then torn > down and reconfigured to shared, the wq configured end up being dedicated > anyays. The root cause is when idxd_device_wqs_clear_state() gets called > during idxd_driver removal, idxd_wq_disable_cleanup() does not get called > vs when the wq driver is removed first. The check of wq state being > "enabled" causes the cleanup to be bypassed. However, idxd_driver->remove() > releases all wq drivers. So the wqs goes to "disabled" state and will never > be "enabled". By that point, the driver has no idea if the wq was > previously configured or clean. So force call idxd_wq_disable_cleanup() on > all wqs always to make sure everything gets cleaned up. > > Reported-by: Tony Zhu <tony.zhu@intel.com> > Tested-by: Tony Zhu <tony.zhu@intel.com> > Fixes: 0dcfe41e9a4c ("dmanegine: idxd: cleanup all device related bits after disabling device") > Signed-off-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > --- > Change Log: > v2: > - Re-based to 5.19-rc2 so that it can be applied cleanly. No functionality > change. > > v1: > https://patchwork.kernel.org/project/linux-dmaengine/patch/165090959239.1376825.18183942742142655091.stgit@djiang5-desk3.ch.intel.com/ > > drivers/dma/idxd/device.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c > index ff0ea60051f0..5a8cc52c1abf 100644 > --- a/drivers/dma/idxd/device.c > +++ b/drivers/dma/idxd/device.c > @@ -716,10 +716,7 @@ static void idxd_device_wqs_clear_state(struct idxd_device *idxd) > struct idxd_wq *wq = idxd->wqs[i]; > > mutex_lock(&wq->wq_lock); > - if (wq->state == IDXD_WQ_ENABLED) { > - idxd_wq_disable_cleanup(wq); > - wq->state = IDXD_WQ_DISABLED; > - } > + idxd_wq_disable_cleanup(wq); > idxd_wq_device_reset_cleanup(wq); > mutex_unlock(&wq->wq_lock); > } > -- > 2.32.0 >
On 15-06-22, 17:53, Fenghua Yu wrote: > On Wed, Jun 15, 2022 at 04:42:19PM -0700, Fenghua Yu wrote: > > From: Dave Jiang <dave.jiang@intel.com> > > > > Testing shown that when a wq mode is setup to be dedicated and then torn > > down and reconfigured to shared, the wq configured end up being dedicated > > anyays. The root cause is when idxd_device_wqs_clear_state() gets called > > during idxd_driver removal, idxd_wq_disable_cleanup() does not get called > > vs when the wq driver is removed first. The check of wq state being > > "enabled" causes the cleanup to be bypassed. However, idxd_driver->remove() > > releases all wq drivers. So the wqs goes to "disabled" state and will never > > be "enabled". By that point, the driver has no idea if the wq was > > previously configured or clean. So force call idxd_wq_disable_cleanup() on > > all wqs always to make sure everything gets cleaned up. > > > > Reported-by: Tony Zhu <tony.zhu@intel.com> > > Tested-by: Tony Zhu <tony.zhu@intel.com> > > Fixes: 0dcfe41e9a4c ("dmanegine: idxd: cleanup all device related bits after disabling device") > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> Pls send updated patchset > > > --- > > Change Log: > > v2: > > - Re-based to 5.19-rc2 so that it can be applied cleanly. No functionality > > change. > > > > v1: > > https://patchwork.kernel.org/project/linux-dmaengine/patch/165090959239.1376825.18183942742142655091.stgit@djiang5-desk3.ch.intel.com/ > > > > drivers/dma/idxd/device.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c > > index ff0ea60051f0..5a8cc52c1abf 100644 > > --- a/drivers/dma/idxd/device.c > > +++ b/drivers/dma/idxd/device.c > > @@ -716,10 +716,7 @@ static void idxd_device_wqs_clear_state(struct idxd_device *idxd) > > struct idxd_wq *wq = idxd->wqs[i]; > > > > mutex_lock(&wq->wq_lock); > > - if (wq->state == IDXD_WQ_ENABLED) { > > - idxd_wq_disable_cleanup(wq); > > - wq->state = IDXD_WQ_DISABLED; > > - } > > + idxd_wq_disable_cleanup(wq); > > idxd_wq_device_reset_cleanup(wq); > > mutex_unlock(&wq->wq_lock); > > } > > -- > > 2.32.0 > >
diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index ff0ea60051f0..5a8cc52c1abf 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -716,10 +716,7 @@ static void idxd_device_wqs_clear_state(struct idxd_device *idxd) struct idxd_wq *wq = idxd->wqs[i]; mutex_lock(&wq->wq_lock); - if (wq->state == IDXD_WQ_ENABLED) { - idxd_wq_disable_cleanup(wq); - wq->state = IDXD_WQ_DISABLED; - } + idxd_wq_disable_cleanup(wq); idxd_wq_device_reset_cleanup(wq); mutex_unlock(&wq->wq_lock); }