| Message ID | 20230712174220.3434989-1-fenghua.yu@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] dmaengine: idxd: Clear PRS disable flag when disabling IDXD device | expand |
On 7/12/23 10:42, Fenghua Yu wrote: > Disabling IDXD device doesn't reset Page Request Service (PRS) > disable flag to its initial value 0. This may cause user confusion > because once PRS is disabled user will see PRS still remains the > previous setting (i.e. disabled) via sysfs interface even after the > device is disabled. > > To eliminate the confusion, reset PRS disable flag when the device > is disabled. > > Tested-by: Tony Zhu <tony.zhu@intel.com> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> Should there be a Fixes tag? > --- > v2: > - Fix Tony's email typo > > drivers/dma/idxd/device.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c > index 5abbcc61c528..71dfb2c13066 100644 > --- a/drivers/dma/idxd/device.c > +++ b/drivers/dma/idxd/device.c > @@ -387,6 +387,7 @@ static void idxd_wq_disable_cleanup(struct idxd_wq *wq) > clear_bit(WQ_FLAG_DEDICATED, &wq->flags); > clear_bit(WQ_FLAG_BLOCK_ON_FAULT, &wq->flags); > clear_bit(WQ_FLAG_ATS_DISABLE, &wq->flags); > + clear_bit(WQ_FLAG_PRS_DISABLE, &wq->flags); I wonder if it's better if we just do wq->flags = 0? I don't see any bits we need to preserve. Do you? > memset(wq->name, 0, WQ_NAME_SIZE); > wq->max_xfer_bytes = WQ_DEFAULT_MAX_XFER; > idxd_wq_set_max_batch_size(idxd->data->type, wq, WQ_DEFAULT_MAX_BATCH);
Hi, Dave, On 7/12/23 10:58, Dave Jiang wrote: > > > On 7/12/23 10:42, Fenghua Yu wrote: >> Disabling IDXD device doesn't reset Page Request Service (PRS) >> disable flag to its initial value 0. This may cause user confusion >> because once PRS is disabled user will see PRS still remains the >> previous setting (i.e. disabled) via sysfs interface even after the >> device is disabled. >> >> To eliminate the confusion, reset PRS disable flag when the device >> is disabled. >> >> Tested-by: Tony Zhu <tony.zhu@intel.com> >> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > > Should there be a Fixes tag? Will add a Fixes tag. >> --- >> v2: >> - Fix Tony's email typo >> >> drivers/dma/idxd/device.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c >> index 5abbcc61c528..71dfb2c13066 100644 >> --- a/drivers/dma/idxd/device.c >> +++ b/drivers/dma/idxd/device.c >> @@ -387,6 +387,7 @@ static void idxd_wq_disable_cleanup(struct idxd_wq >> *wq) >> clear_bit(WQ_FLAG_DEDICATED, &wq->flags); >> clear_bit(WQ_FLAG_BLOCK_ON_FAULT, &wq->flags); >> clear_bit(WQ_FLAG_ATS_DISABLE, &wq->flags); >> + clear_bit(WQ_FLAG_PRS_DISABLE, &wq->flags); > > I wonder if it's better if we just do wq->flags = 0? I don't see any > bits we need to preserve. Do you? wq->flags = 0 is better. Thanks. -Fenghua
diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index 5abbcc61c528..71dfb2c13066 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -387,6 +387,7 @@ static void idxd_wq_disable_cleanup(struct idxd_wq *wq) clear_bit(WQ_FLAG_DEDICATED, &wq->flags); clear_bit(WQ_FLAG_BLOCK_ON_FAULT, &wq->flags); clear_bit(WQ_FLAG_ATS_DISABLE, &wq->flags); + clear_bit(WQ_FLAG_PRS_DISABLE, &wq->flags); memset(wq->name, 0, WQ_NAME_SIZE); wq->max_xfer_bytes = WQ_DEFAULT_MAX_XFER; idxd_wq_set_max_batch_size(idxd->data->type, wq, WQ_DEFAULT_MAX_BATCH);