| Message ID | 20240603215558.2722969-9-aahringo@redhat.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | dlm: md: introduce DLM_LSFL_SOFTIRQ_SAFE | expand |
Hi, On Mon, Jun 3, 2024 at 5:56 PM Alexander Aring <aahringo@redhat.com> wrote: > > Recently the DLM subsystem introduced the flag DLM_LSFL_SOFTIRQ for > dlm_new_lockspace() to signal the capability to handle DLM ast/bast > callbacks in softirq context to avoid an additional context switch due > the DLM callback workqueue. > > The md-cluster implementation only does synchronized calls above the > async DLM API. That synchronized API should may be also offered by DLM, > however it is very simple as md-cluster callbacks only does a complete() > call for their wait_for_completion() wait that is occurred after the > async DLM API call. This patch activates the recently introduced > DLM_LSFL_SOFTIRQ flag that allows that the DLM callbacks are executed in > a softirq context that md-cluster can handle. It is reducing a > unnecessary context workqueue switch and should speed up DLM in some > circumstance. > Can somebody with a md-cluster environment test it as well? All I was doing was (with a working dlm_controld cluster env): mdadm --create /dev/md0 --bitmap=clustered --metadata=1.2 --raid-devices=2 --level=mirror /dev/sda /dev/sdb sda and sdb are shared block devices on each node. Create a /etc/mdadm.conf with the content mostly out of: mdadm --detail --scan on each node. Then call mdadm --assemble on all nodes where not "mdadm --create ..." happened. I hope that is the right thing to do and I had with "dlm_tool ls" a UUID as a lockspace name with some md-cluster locks being around. To bring this new flag upstream, would it be okay to get this through dlm-tree? I am requesting here for an "Acked-by" tag from the md maintainers. Thanks. - Alex
On 6/6/24 02:54, Alexander Aring wrote: > Hi, > > On Mon, Jun 3, 2024 at 5:56 PM Alexander Aring <aahringo@redhat.com> wrote: >> >> Recently the DLM subsystem introduced the flag DLM_LSFL_SOFTIRQ for >> dlm_new_lockspace() to signal the capability to handle DLM ast/bast >> callbacks in softirq context to avoid an additional context switch due >> the DLM callback workqueue. >> >> The md-cluster implementation only does synchronized calls above the >> async DLM API. That synchronized API should may be also offered by DLM, >> however it is very simple as md-cluster callbacks only does a complete() >> call for their wait_for_completion() wait that is occurred after the >> async DLM API call. This patch activates the recently introduced >> DLM_LSFL_SOFTIRQ flag that allows that the DLM callbacks are executed in >> a softirq context that md-cluster can handle. It is reducing a >> unnecessary context workqueue switch and should speed up DLM in some >> circumstance. >> > > Can somebody with a md-cluster environment test it as well? All I was > doing was (with a working dlm_controld cluster env): > > mdadm --create /dev/md0 --bitmap=clustered --metadata=1.2 > --raid-devices=2 --level=mirror /dev/sda /dev/sdb > > sda and sdb are shared block devices on each node. > > Create a /etc/mdadm.conf with the content mostly out of: > > mdadm --detail --scan > > on each node. > > Then call mdadm --assemble on all nodes where not "mdadm --create ..." happened. > I hope that is the right thing to do and I had with "dlm_tool ls" a > UUID as a lockspace name with some md-cluster locks being around. The above setup method is correct. SUSE doc [1] provides more details on assembling the clustered array. [1]: https://documentation.suse.com/fr-fr/sle-ha/15-SP5/html/SLE-HA-all/cha-ha-cluster-md.html#sec-ha-cluster-md-create > > To bring this new flag upstream, would it be okay to get this through > dlm-tree? I am requesting here for an "Acked-by" tag from the md > maintainers. > I compiled & tested the dlm-tree [2] with SUSE CI env, and didn't see these patches introduce new issue. [2]: https://git.kernel.org/pub/scm/linux/kernel/git/teigland/linux-dlm.git/log/?h=next Thanks, Heming
Hi Heming, On Wed, Jun 5, 2024 at 10:48 PM Heming Zhao <heming.zhao@suse.com> wrote: > > On 6/6/24 02:54, Alexander Aring wrote: > > Hi, > > > > On Mon, Jun 3, 2024 at 5:56 PM Alexander Aring <aahringo@redhat.com> wrote: > >> > >> Recently the DLM subsystem introduced the flag DLM_LSFL_SOFTIRQ for > >> dlm_new_lockspace() to signal the capability to handle DLM ast/bast > >> callbacks in softirq context to avoid an additional context switch due > >> the DLM callback workqueue. > >> > >> The md-cluster implementation only does synchronized calls above the > >> async DLM API. That synchronized API should may be also offered by DLM, > >> however it is very simple as md-cluster callbacks only does a complete() > >> call for their wait_for_completion() wait that is occurred after the > >> async DLM API call. This patch activates the recently introduced > >> DLM_LSFL_SOFTIRQ flag that allows that the DLM callbacks are executed in > >> a softirq context that md-cluster can handle. It is reducing a > >> unnecessary context workqueue switch and should speed up DLM in some > >> circumstance. > >> > > > > Can somebody with a md-cluster environment test it as well? All I was > > doing was (with a working dlm_controld cluster env): > > > > mdadm --create /dev/md0 --bitmap=clustered --metadata=1.2 > > --raid-devices=2 --level=mirror /dev/sda /dev/sdb > > > > sda and sdb are shared block devices on each node. > > > > Create a /etc/mdadm.conf with the content mostly out of: > > > > mdadm --detail --scan > > > > on each node. > > > > Then call mdadm --assemble on all nodes where not "mdadm --create ..." happened. > > I hope that is the right thing to do and I had with "dlm_tool ls" a > > UUID as a lockspace name with some md-cluster locks being around. > > The above setup method is correct. > SUSE doc [1] provides more details on assembling the clustered array. > yea, I saw that and mostly cut it down into the necessary steps in my development setup. Thanks for confirming I did something right here. > [1]: https://documentation.suse.com/fr-fr/sle-ha/15-SP5/html/SLE-HA-all/cha-ha-cluster-md.html#sec-ha-cluster-md-create > > > > > To bring this new flag upstream, would it be okay to get this through > > dlm-tree? I am requesting here for an "Acked-by" tag from the md > > maintainers. > > > > I compiled & tested the dlm-tree [2] with SUSE CI env, and didn't see these > patches introduce new issue. > Thanks for doing that. So that means you tried the dlm-tree with this patch series applied? Song or Yu, can I get an "Acked-by" from you and an answer if it is okay that this md-cluster.c patch goes upstream via dlm-tree? Thanks. - Alex
On 6/6/24 22:33, Alexander Aring wrote: > Hi Heming, > > On Wed, Jun 5, 2024 at 10:48 PM Heming Zhao <heming.zhao@suse.com> wrote: >> >> On 6/6/24 02:54, Alexander Aring wrote: >>> Hi, >>> >>> On Mon, Jun 3, 2024 at 5:56 PM Alexander Aring <aahringo@redhat.com> wrote: >>>> >>>> Recently the DLM subsystem introduced the flag DLM_LSFL_SOFTIRQ for >>>> dlm_new_lockspace() to signal the capability to handle DLM ast/bast >>>> callbacks in softirq context to avoid an additional context switch due >>>> the DLM callback workqueue. >>>> >>>> The md-cluster implementation only does synchronized calls above the >>>> async DLM API. That synchronized API should may be also offered by DLM, >>>> however it is very simple as md-cluster callbacks only does a complete() >>>> call for their wait_for_completion() wait that is occurred after the >>>> async DLM API call. This patch activates the recently introduced >>>> DLM_LSFL_SOFTIRQ flag that allows that the DLM callbacks are executed in >>>> a softirq context that md-cluster can handle. It is reducing a >>>> unnecessary context workqueue switch and should speed up DLM in some >>>> circumstance. >>>> >>> >>> Can somebody with a md-cluster environment test it as well? All I was >>> doing was (with a working dlm_controld cluster env): >>> >>> mdadm --create /dev/md0 --bitmap=clustered --metadata=1.2 >>> --raid-devices=2 --level=mirror /dev/sda /dev/sdb >>> >>> sda and sdb are shared block devices on each node. >>> >>> Create a /etc/mdadm.conf with the content mostly out of: >>> >>> mdadm --detail --scan >>> >>> on each node. >>> >>> Then call mdadm --assemble on all nodes where not "mdadm --create ..." happened. >>> I hope that is the right thing to do and I had with "dlm_tool ls" a >>> UUID as a lockspace name with some md-cluster locks being around. >> >> The above setup method is correct. >> SUSE doc [1] provides more details on assembling the clustered array. >> > > yea, I saw that and mostly cut it down into the necessary steps in my > development setup. > > Thanks for confirming I did something right here. > >> [1]: https://documentation.suse.com/fr-fr/sle-ha/15-SP5/html/SLE-HA-all/cha-ha-cluster-md.html#sec-ha-cluster-md-create >> >>> >>> To bring this new flag upstream, would it be okay to get this through >>> dlm-tree? I am requesting here for an "Acked-by" tag from the md >>> maintainers. >>> >> >> I compiled & tested the dlm-tree [2] with SUSE CI env, and didn't see these >> patches introduce new issue. >> > > Thanks for doing that. So that means you tried the dlm-tree with this > patch series applied? Yes. -Heming > > Song or Yu, can I get an "Acked-by" from you and an answer if it is > okay that this md-cluster.c patch goes upstream via dlm-tree? > > Thanks. > > - Alex >
On Thu, Jun 6, 2024 at 7:34 AM Alexander Aring <aahringo@redhat.com> wrote: > > Hi Heming, > > On Wed, Jun 5, 2024 at 10:48 PM Heming Zhao <heming.zhao@suse.com> wrote: > > > > On 6/6/24 02:54, Alexander Aring wrote: > > > Hi, > > > > > > On Mon, Jun 3, 2024 at 5:56 PM Alexander Aring <aahringo@redhat.com> wrote: > > >> > > >> Recently the DLM subsystem introduced the flag DLM_LSFL_SOFTIRQ for > > >> dlm_new_lockspace() to signal the capability to handle DLM ast/bast > > >> callbacks in softirq context to avoid an additional context switch due > > >> the DLM callback workqueue. > > >> > > >> The md-cluster implementation only does synchronized calls above the > > >> async DLM API. That synchronized API should may be also offered by DLM, > > >> however it is very simple as md-cluster callbacks only does a complete() > > >> call for their wait_for_completion() wait that is occurred after the > > >> async DLM API call. This patch activates the recently introduced > > >> DLM_LSFL_SOFTIRQ flag that allows that the DLM callbacks are executed in > > >> a softirq context that md-cluster can handle. It is reducing a > > >> unnecessary context workqueue switch and should speed up DLM in some > > >> circumstance. > > >> > > > > > > Can somebody with a md-cluster environment test it as well? All I was > > > doing was (with a working dlm_controld cluster env): > > > > > > mdadm --create /dev/md0 --bitmap=clustered --metadata=1.2 > > > --raid-devices=2 --level=mirror /dev/sda /dev/sdb > > > > > > sda and sdb are shared block devices on each node. > > > > > > Create a /etc/mdadm.conf with the content mostly out of: > > > > > > mdadm --detail --scan > > > > > > on each node. > > > > > > Then call mdadm --assemble on all nodes where not "mdadm --create ..." happened. > > > I hope that is the right thing to do and I had with "dlm_tool ls" a > > > UUID as a lockspace name with some md-cluster locks being around. > > > > The above setup method is correct. > > SUSE doc [1] provides more details on assembling the clustered array. > > > > yea, I saw that and mostly cut it down into the necessary steps in my > development setup. > > Thanks for confirming I did something right here. > > > [1]: https://documentation.suse.com/fr-fr/sle-ha/15-SP5/html/SLE-HA-all/cha-ha-cluster-md.html#sec-ha-cluster-md-create > > > > > > > > To bring this new flag upstream, would it be okay to get this through > > > dlm-tree? I am requesting here for an "Acked-by" tag from the md > > > maintainers. > > > > > > > I compiled & tested the dlm-tree [2] with SUSE CI env, and didn't see these > > patches introduce new issue. > > > > Thanks for doing that. So that means you tried the dlm-tree with this > patch series applied? > > Song or Yu, can I get an "Acked-by" from you and an answer if it is > okay that this md-cluster.c patch goes upstream via dlm-tree? LGTM. Acked-by: Song Liu <song@kernel.org> Yes, let's route this via dlm-tree. Thanks, Song
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c index 8e36a0feec09..eb9bbf12c8d8 100644 --- a/drivers/md/md-cluster.c +++ b/drivers/md/md-cluster.c @@ -887,7 +887,7 @@ static int join(struct mddev *mddev, int nodes) memset(str, 0, 64); sprintf(str, "%pU", mddev->uuid); ret = dlm_new_lockspace(str, mddev->bitmap_info.cluster_name, - 0, LVB_SIZE, &md_ls_ops, mddev, + DLM_LSFL_SOFTIRQ, LVB_SIZE, &md_ls_ops, mddev, &ops_rv, &cinfo->lockspace); if (ret) goto err;
Recently the DLM subsystem introduced the flag DLM_LSFL_SOFTIRQ for dlm_new_lockspace() to signal the capability to handle DLM ast/bast callbacks in softirq context to avoid an additional context switch due the DLM callback workqueue. The md-cluster implementation only does synchronized calls above the async DLM API. That synchronized API should may be also offered by DLM, however it is very simple as md-cluster callbacks only does a complete() call for their wait_for_completion() wait that is occurred after the async DLM API call. This patch activates the recently introduced DLM_LSFL_SOFTIRQ flag that allows that the DLM callbacks are executed in a softirq context that md-cluster can handle. It is reducing a unnecessary context workqueue switch and should speed up DLM in some circumstance. In future other DLM users e.g. gfs2 will also take usage of this flag to avoid the additional context switch due the DLM callback workqueue. In far future hopefully we can remove this kernel lockspace flag only and remove the callback workqueue at all. Signed-off-by: Alexander Aring <aahringo@redhat.com> --- drivers/md/md-cluster.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)