* [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues
@ 2022-09-28 15:48 Jerry Snitselaar
2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jerry Snitselaar @ 2022-09-28 15:48 UTC (permalink / raw)
To: linux-kernel, dmaengine; +Cc: Fenghua Yu, Dave Jiang, Vinod Koul
Currently if a software reset is attempted on an idxd device
the workqueues will not be re-enabled, because it will see
incorrectly see that wq->state is already set to IDXD_WQ_ENABLED.
So set the workqueue state to disabled in idxd_wq_disable_cleanup(),
and use a bitmap to track which workqueues have been enabled so they
can be re-enabled during device re-initialization.
Changes from v1 to v2:
- Clear bit in case where idxd_wq_enable() fails during re-init.
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Jerry Snitselaar (2):
dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup()
dmaengine: idxd: track enabled workqueues in bitmap
drivers/dma/idxd/device.c | 4 +++-
drivers/dma/idxd/idxd.h | 2 ++
drivers/dma/idxd/init.c | 6 ++++++
drivers/dma/idxd/irq.c | 5 +++--
drivers/dma/idxd/sysfs.c | 1 +
5 files changed, 15 insertions(+), 3 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() 2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar @ 2022-09-28 15:48 ` Jerry Snitselaar 2022-09-28 15:54 ` Dave Jiang 2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar 2022-09-29 7:32 ` [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Vinod Koul 2 siblings, 1 reply; 6+ messages in thread From: Jerry Snitselaar @ 2022-09-28 15:48 UTC (permalink / raw) To: linux-kernel, dmaengine; +Cc: Fenghua Yu, Dave Jiang, Vinod Koul If we are calling idxd_wq_disable_cleanup(), the workqueue should be in a disabled state. So set the workqueue state to IDXD_WQ_DISABLED so that the state reflects that. Currently if there is a device failure, and a software reset is attempted the workqueues will not be re-enabled due to idxd_wq_enable() seeing that state as already being IDXD_WQ_ENABLED. Cc: Fenghua Yu <fenghua.yu@intel.com> Cc: Dave Jiang <dave.jiang@intel.com> Cc: Vinod Koul <vkoul@kernel.org> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> --- drivers/dma/idxd/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index 5a8cc52c1abf..31911e255ac1 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -258,7 +258,6 @@ void idxd_wq_reset(struct idxd_wq *wq) operand = BIT(wq->id % 16) | ((wq->id / 16) << 16); idxd_cmd_exec(idxd, IDXD_CMD_RESET_WQ, operand, NULL); idxd_wq_disable_cleanup(wq); - wq->state = IDXD_WQ_DISABLED; } int idxd_wq_map_portal(struct idxd_wq *wq) @@ -378,6 +377,7 @@ static void idxd_wq_disable_cleanup(struct idxd_wq *wq) struct idxd_device *idxd = wq->idxd; lockdep_assert_held(&wq->wq_lock); + wq->state = IDXD_WQ_DISABLED; memset(wq->wqcfg, 0, idxd->wqcfg_size); wq->type = IDXD_WQT_NONE; wq->threshold = 0; -- 2.37.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() 2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar @ 2022-09-28 15:54 ` Dave Jiang 0 siblings, 0 replies; 6+ messages in thread From: Dave Jiang @ 2022-09-28 15:54 UTC (permalink / raw) To: Jerry Snitselaar, linux-kernel, dmaengine; +Cc: Fenghua Yu, Vinod Koul On 9/28/2022 8:48 AM, Jerry Snitselaar wrote: > If we are calling idxd_wq_disable_cleanup(), the workqueue should be > in a disabled state. So set the workqueue state to IDXD_WQ_DISABLED so > that the state reflects that. Currently if there is a device failure, > and a software reset is attempted the workqueues will not be > re-enabled due to idxd_wq_enable() seeing that state as already being > IDXD_WQ_ENABLED. > > Cc: Fenghua Yu <fenghua.yu@intel.com> > Cc: Dave Jiang <dave.jiang@intel.com> > Cc: Vinod Koul <vkoul@kernel.org> > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/dma/idxd/device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c > index 5a8cc52c1abf..31911e255ac1 100644 > --- a/drivers/dma/idxd/device.c > +++ b/drivers/dma/idxd/device.c > @@ -258,7 +258,6 @@ void idxd_wq_reset(struct idxd_wq *wq) > operand = BIT(wq->id % 16) | ((wq->id / 16) << 16); > idxd_cmd_exec(idxd, IDXD_CMD_RESET_WQ, operand, NULL); > idxd_wq_disable_cleanup(wq); > - wq->state = IDXD_WQ_DISABLED; > } > > int idxd_wq_map_portal(struct idxd_wq *wq) > @@ -378,6 +377,7 @@ static void idxd_wq_disable_cleanup(struct idxd_wq *wq) > struct idxd_device *idxd = wq->idxd; > > lockdep_assert_held(&wq->wq_lock); > + wq->state = IDXD_WQ_DISABLED; > memset(wq->wqcfg, 0, idxd->wqcfg_size); > wq->type = IDXD_WQT_NONE; > wq->threshold = 0; ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap 2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar 2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar @ 2022-09-28 15:48 ` Jerry Snitselaar 2022-09-28 15:51 ` Dave Jiang 2022-09-29 7:32 ` [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Vinod Koul 2 siblings, 1 reply; 6+ messages in thread From: Jerry Snitselaar @ 2022-09-28 15:48 UTC (permalink / raw) To: linux-kernel, dmaengine; +Cc: Fenghua Yu, Dave Jiang, Vinod Koul Now that idxd_wq_disable_cleanup() sets the workqueue state to IDXD_WQ_DISABLED, use a bitmap to track which workqueues have been enabled. This will then be used to determine which workqueues should be re-enabled when attempting a software reset to recover from a device halt state. Cc: Fenghua Yu <fenghua.yu@intel.com> Cc: Dave Jiang <dave.jiang@intel.com> Cc: Vinod Koul <vkoul@kernel.org> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> --- v2: Clear bit in case where idxd_wq_enable fails during re-init drivers/dma/idxd/device.c | 2 ++ drivers/dma/idxd/idxd.h | 2 ++ drivers/dma/idxd/init.c | 6 ++++++ drivers/dma/idxd/irq.c | 5 +++-- drivers/dma/idxd/sysfs.c | 1 + 5 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index 31911e255ac1..f0c7d6d348e3 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -196,6 +196,7 @@ int idxd_wq_enable(struct idxd_wq *wq) } wq->state = IDXD_WQ_ENABLED; + set_bit(wq->id, idxd->wq_enable_map); dev_dbg(dev, "WQ %d enabled\n", wq->id); return 0; } @@ -223,6 +224,7 @@ int idxd_wq_disable(struct idxd_wq *wq, bool reset_config) if (reset_config) idxd_wq_disable_cleanup(wq); + clear_bit(wq->id, idxd->wq_enable_map); wq->state = IDXD_WQ_DISABLED; dev_dbg(dev, "WQ %d disabled\n", wq->id); return 0; diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h index fed0dfc1eaa8..f527a7f88b92 100644 --- a/drivers/dma/idxd/idxd.h +++ b/drivers/dma/idxd/idxd.h @@ -11,6 +11,7 @@ #include <linux/idr.h> #include <linux/pci.h> #include <linux/ioasid.h> +#include <linux/bitmap.h> #include <linux/perf_event.h> #include <uapi/linux/idxd.h> #include "registers.h" @@ -299,6 +300,7 @@ struct idxd_device { int rdbuf_limit; int nr_rdbufs; /* non-reserved read buffers */ unsigned int wqcfg_size; + unsigned long *wq_enable_map; union sw_err_reg sw_err; wait_queue_head_t cmd_waitq; diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c index aa3478257ddb..7e27e69ff741 100644 --- a/drivers/dma/idxd/init.c +++ b/drivers/dma/idxd/init.c @@ -151,6 +151,12 @@ static int idxd_setup_wqs(struct idxd_device *idxd) if (!idxd->wqs) return -ENOMEM; + idxd->wq_enable_map = bitmap_zalloc_node(idxd->max_wqs, GFP_KERNEL, dev_to_node(dev)); + if (!idxd->wq_enable_map) { + kfree(idxd->wqs); + return -ENOMEM; + } + for (i = 0; i < idxd->max_wqs; i++) { wq = kzalloc_node(sizeof(*wq), GFP_KERNEL, dev_to_node(dev)); if (!wq) { diff --git a/drivers/dma/idxd/irq.c b/drivers/dma/idxd/irq.c index 743ead5ebc57..3fcfbb7bf6e3 100644 --- a/drivers/dma/idxd/irq.c +++ b/drivers/dma/idxd/irq.c @@ -49,11 +49,12 @@ static void idxd_device_reinit(struct work_struct *work) goto out; for (i = 0; i < idxd->max_wqs; i++) { - struct idxd_wq *wq = idxd->wqs[i]; + if (test_bit(i, idxd->wq_enable_map)) { + struct idxd_wq *wq = idxd->wqs[i]; - if (wq->state == IDXD_WQ_ENABLED) { rc = idxd_wq_enable(wq); if (rc < 0) { + clear_bit(i, idxd->wq_enable_map); dev_warn(dev, "Unable to re-enable wq %s\n", dev_name(wq_confdev(wq))); } diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c index 3f262a57441b..3325b16ed959 100644 --- a/drivers/dma/idxd/sysfs.c +++ b/drivers/dma/idxd/sysfs.c @@ -1405,6 +1405,7 @@ static void idxd_conf_device_release(struct device *dev) struct idxd_device *idxd = confdev_to_idxd(dev); kfree(idxd->groups); + bitmap_free(idxd->wq_enable_map); kfree(idxd->wqs); kfree(idxd->engines); ida_free(&idxd_ida, idxd->id); -- 2.37.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap 2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar @ 2022-09-28 15:51 ` Dave Jiang 0 siblings, 0 replies; 6+ messages in thread From: Dave Jiang @ 2022-09-28 15:51 UTC (permalink / raw) To: Jerry Snitselaar, linux-kernel, dmaengine; +Cc: Fenghua Yu, Vinod Koul On 9/28/2022 8:48 AM, Jerry Snitselaar wrote: > Now that idxd_wq_disable_cleanup() sets the workqueue state to > IDXD_WQ_DISABLED, use a bitmap to track which workqueues have been > enabled. This will then be used to determine which workqueues > should be re-enabled when attempting a software reset to recover > from a device halt state. > > Cc: Fenghua Yu <fenghua.yu@intel.com> > Cc: Dave Jiang <dave.jiang@intel.com> > Cc: Vinod Koul <vkoul@kernel.org> > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > v2: Clear bit in case where idxd_wq_enable fails during re-init > > drivers/dma/idxd/device.c | 2 ++ > drivers/dma/idxd/idxd.h | 2 ++ > drivers/dma/idxd/init.c | 6 ++++++ > drivers/dma/idxd/irq.c | 5 +++-- > drivers/dma/idxd/sysfs.c | 1 + > 5 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c > index 31911e255ac1..f0c7d6d348e3 100644 > --- a/drivers/dma/idxd/device.c > +++ b/drivers/dma/idxd/device.c > @@ -196,6 +196,7 @@ int idxd_wq_enable(struct idxd_wq *wq) > } > > wq->state = IDXD_WQ_ENABLED; > + set_bit(wq->id, idxd->wq_enable_map); > dev_dbg(dev, "WQ %d enabled\n", wq->id); > return 0; > } > @@ -223,6 +224,7 @@ int idxd_wq_disable(struct idxd_wq *wq, bool reset_config) > > if (reset_config) > idxd_wq_disable_cleanup(wq); > + clear_bit(wq->id, idxd->wq_enable_map); > wq->state = IDXD_WQ_DISABLED; > dev_dbg(dev, "WQ %d disabled\n", wq->id); > return 0; > diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h > index fed0dfc1eaa8..f527a7f88b92 100644 > --- a/drivers/dma/idxd/idxd.h > +++ b/drivers/dma/idxd/idxd.h > @@ -11,6 +11,7 @@ > #include <linux/idr.h> > #include <linux/pci.h> > #include <linux/ioasid.h> > +#include <linux/bitmap.h> > #include <linux/perf_event.h> > #include <uapi/linux/idxd.h> > #include "registers.h" > @@ -299,6 +300,7 @@ struct idxd_device { > int rdbuf_limit; > int nr_rdbufs; /* non-reserved read buffers */ > unsigned int wqcfg_size; > + unsigned long *wq_enable_map; > > union sw_err_reg sw_err; > wait_queue_head_t cmd_waitq; > diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c > index aa3478257ddb..7e27e69ff741 100644 > --- a/drivers/dma/idxd/init.c > +++ b/drivers/dma/idxd/init.c > @@ -151,6 +151,12 @@ static int idxd_setup_wqs(struct idxd_device *idxd) > if (!idxd->wqs) > return -ENOMEM; > > + idxd->wq_enable_map = bitmap_zalloc_node(idxd->max_wqs, GFP_KERNEL, dev_to_node(dev)); > + if (!idxd->wq_enable_map) { > + kfree(idxd->wqs); > + return -ENOMEM; > + } > + > for (i = 0; i < idxd->max_wqs; i++) { > wq = kzalloc_node(sizeof(*wq), GFP_KERNEL, dev_to_node(dev)); > if (!wq) { > diff --git a/drivers/dma/idxd/irq.c b/drivers/dma/idxd/irq.c > index 743ead5ebc57..3fcfbb7bf6e3 100644 > --- a/drivers/dma/idxd/irq.c > +++ b/drivers/dma/idxd/irq.c > @@ -49,11 +49,12 @@ static void idxd_device_reinit(struct work_struct *work) > goto out; > > for (i = 0; i < idxd->max_wqs; i++) { > - struct idxd_wq *wq = idxd->wqs[i]; > + if (test_bit(i, idxd->wq_enable_map)) { > + struct idxd_wq *wq = idxd->wqs[i]; > > - if (wq->state == IDXD_WQ_ENABLED) { > rc = idxd_wq_enable(wq); > if (rc < 0) { > + clear_bit(i, idxd->wq_enable_map); > dev_warn(dev, "Unable to re-enable wq %s\n", > dev_name(wq_confdev(wq))); > } > diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c > index 3f262a57441b..3325b16ed959 100644 > --- a/drivers/dma/idxd/sysfs.c > +++ b/drivers/dma/idxd/sysfs.c > @@ -1405,6 +1405,7 @@ static void idxd_conf_device_release(struct device *dev) > struct idxd_device *idxd = confdev_to_idxd(dev); > > kfree(idxd->groups); > + bitmap_free(idxd->wq_enable_map); > kfree(idxd->wqs); > kfree(idxd->engines); > ida_free(&idxd_ida, idxd->id); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues 2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar 2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar 2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar @ 2022-09-29 7:32 ` Vinod Koul 2 siblings, 0 replies; 6+ messages in thread From: Vinod Koul @ 2022-09-29 7:32 UTC (permalink / raw) To: Jerry Snitselaar; +Cc: linux-kernel, dmaengine, Fenghua Yu, Dave Jiang On 28-09-22, 08:48, Jerry Snitselaar wrote: > Currently if a software reset is attempted on an idxd device > the workqueues will not be re-enabled, because it will see > incorrectly see that wq->state is already set to IDXD_WQ_ENABLED. > So set the workqueue state to disabled in idxd_wq_disable_cleanup(), > and use a bitmap to track which workqueues have been enabled so they > can be re-enabled during device re-initialization. Applied, thanks -- ~Vinod ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-29 7:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar 2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar 2022-09-28 15:54 ` Dave Jiang 2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar 2022-09-28 15:51 ` Dave Jiang 2022-09-29 7:32 ` [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox