| Message ID | 20200417111545.30437-1-bob.liu@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] scsi: iscsi: register sysfs for iscsi workqueue | expand |
Friendly ping.. On 4/17/20 7:15 PM, Bob Liu wrote: > Then users can set cpu affinity through "cpumask" for iscsi workqueues, so > as to get performance isolation. > > This patch changes the max number of active worker form 1 to 2, > since ordered workqueue wont' be allowed to change "cpumask". > > Signed-off-by: Bob Liu <bob.liu@oracle.com> > --- > drivers/scsi/libiscsi.c | 4 +++- > drivers/scsi/scsi_transport_iscsi.c | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c > index 70b99c0..adf9bb4 100644 > --- a/drivers/scsi/libiscsi.c > +++ b/drivers/scsi/libiscsi.c > @@ -2627,7 +2627,9 @@ struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht, > if (xmit_can_sleep) { > snprintf(ihost->workq_name, sizeof(ihost->workq_name), > "iscsi_q_%d", shost->host_no); > - ihost->workq = create_singlethread_workqueue(ihost->workq_name); > + ihost->workq = alloc_workqueue("%s", > + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, > + 2, ihost->workq_name); > if (!ihost->workq) > goto free_host; > } > diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c > index dfc726f..bdbc4a2 100644 > --- a/drivers/scsi/scsi_transport_iscsi.c > +++ b/drivers/scsi/scsi_transport_iscsi.c > @@ -4602,7 +4602,9 @@ static __init int iscsi_transport_init(void) > goto unregister_flashnode_bus; > } > > - iscsi_eh_timer_workq = create_singlethread_workqueue("iscsi_eh"); > + iscsi_eh_timer_workq = alloc_workqueue("%s", > + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, > + 2, "iscsi_eh"); > if (!iscsi_eh_timer_workq) { > err = -ENOMEM; > goto release_nls; >
On 17.04.20 13:15, Bob Liu wrote: > Then users can set cpu affinity through "cpumask" for iscsi workqueues, so > as to get performance isolation. > > This patch changes the max number of active worker form 1 to 2, > since ordered workqueue wont' be allowed to change "cpumask". > Won't having 2 workers break the current ordering guarantees? Did you check that no-one depends on this? > Signed-off-by: Bob Liu <bob.liu@oracle.com> > --- > drivers/scsi/libiscsi.c | 4 +++- > drivers/scsi/scsi_transport_iscsi.c | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c > index 70b99c0..adf9bb4 100644 > --- a/drivers/scsi/libiscsi.c > +++ b/drivers/scsi/libiscsi.c > @@ -2627,7 +2627,9 @@ struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht, > if (xmit_can_sleep) { > snprintf(ihost->workq_name, sizeof(ihost->workq_name), > "iscsi_q_%d", shost->host_no); > - ihost->workq = create_singlethread_workqueue(ihost->workq_name); > + ihost->workq = alloc_workqueue("%s", > + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, > + 2, ihost->workq_name); From looking at the documentation, __WQ_LEGACY doesn't seem to belong here. > if (!ihost->workq) > goto free_host; > } > diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c > index dfc726f..bdbc4a2 100644 > --- a/drivers/scsi/scsi_transport_iscsi.c > +++ b/drivers/scsi/scsi_transport_iscsi.c > @@ -4602,7 +4602,9 @@ static __init int iscsi_transport_init(void) > goto unregister_flashnode_bus; > } > > - iscsi_eh_timer_workq = create_singlethread_workqueue("iscsi_eh"); > + iscsi_eh_timer_workq = alloc_workqueue("%s", > + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, > + 2, "iscsi_eh"); > if (!iscsi_eh_timer_workq) { > err = -ENOMEM; > goto release_nls; >
Sorry forgot Cc iscsi maintainers. On 4/28/20 6:56 PM, Julian Wiedmann wrote: > On 17.04.20 13:15, Bob Liu wrote: >> Then users can set cpu affinity through "cpumask" for iscsi workqueues, so >> as to get performance isolation. >> >> This patch changes the max number of active worker form 1 to 2, >> since ordered workqueue wont' be allowed to change "cpumask". >> > > Won't having 2 workers break the current ordering guarantees? Yes. > Did you check that no-one depends on this? > Not quite sure, I'd better add [rfc] to this patch. But this is the easiest way to set cpu affinity of iscsi workqueue. >> Signed-off-by: Bob Liu <bob.liu@oracle.com> >> --- >> drivers/scsi/libiscsi.c | 4 +++- >> drivers/scsi/scsi_transport_iscsi.c | 4 +++- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c >> index 70b99c0..adf9bb4 100644 >> --- a/drivers/scsi/libiscsi.c >> +++ b/drivers/scsi/libiscsi.c >> @@ -2627,7 +2627,9 @@ struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht, >> if (xmit_can_sleep) { >> snprintf(ihost->workq_name, sizeof(ihost->workq_name), >> "iscsi_q_%d", shost->host_no); >> - ihost->workq = create_singlethread_workqueue(ihost->workq_name); >> + ihost->workq = alloc_workqueue("%s", >> + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, >> + 2, ihost->workq_name); > > From looking at the documentation, __WQ_LEGACY doesn't seem to belong here. > Indeed, thanks for the review. >> if (!ihost->workq) >> goto free_host; >> } >> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c >> index dfc726f..bdbc4a2 100644 >> --- a/drivers/scsi/scsi_transport_iscsi.c >> +++ b/drivers/scsi/scsi_transport_iscsi.c >> @@ -4602,7 +4602,9 @@ static __init int iscsi_transport_init(void) >> goto unregister_flashnode_bus; >> } >> >> - iscsi_eh_timer_workq = create_singlethread_workqueue("iscsi_eh"); >> + iscsi_eh_timer_workq = alloc_workqueue("%s", >> + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, >> + 2, "iscsi_eh"); >> if (!iscsi_eh_timer_workq) { >> err = -ENOMEM; >> goto release_nls; >> >
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index 70b99c0..adf9bb4 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -2627,7 +2627,9 @@ struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht, if (xmit_can_sleep) { snprintf(ihost->workq_name, sizeof(ihost->workq_name), "iscsi_q_%d", shost->host_no); - ihost->workq = create_singlethread_workqueue(ihost->workq_name); + ihost->workq = alloc_workqueue("%s", + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, + 2, ihost->workq_name); if (!ihost->workq) goto free_host; } diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index dfc726f..bdbc4a2 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -4602,7 +4602,9 @@ static __init int iscsi_transport_init(void) goto unregister_flashnode_bus; } - iscsi_eh_timer_workq = create_singlethread_workqueue("iscsi_eh"); + iscsi_eh_timer_workq = alloc_workqueue("%s", + WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND, + 2, "iscsi_eh"); if (!iscsi_eh_timer_workq) { err = -ENOMEM; goto release_nls;
Then users can set cpu affinity through "cpumask" for iscsi workqueues, so as to get performance isolation. This patch changes the max number of active worker form 1 to 2, since ordered workqueue wont' be allowed to change "cpumask". Signed-off-by: Bob Liu <bob.liu@oracle.com> --- drivers/scsi/libiscsi.c | 4 +++- drivers/scsi/scsi_transport_iscsi.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)