Re: [dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
Xose Vazquez Perez writes: > On 5/11/20 6:39 PM, Gabriel Krisman Bertazi wrote: > >> This fourth version of HST applies the suggestion from Mikulas Patocka >> to do the ktime_get_ns inside the mpath map_bio instead of generic >> device-mapper code. This means that struct dm_mpath_io gained another >> 64bit field. For the request-based case, we continue to use the block >> layer start time information. > > You should add some info to the multipath.conf.5 man page ( > https://git.opensvc.com/gitweb.cgi?p=multipath-tools/.git;a=blob;f=multipath/multipath.conf.5;h=05a5e8ffeb110d969f3b2381eb3b88d7f28380f6;hb=HEAD#l189 > ), > or none one is going to use it. Sure, will do. -- Gabriel Krisman Bertazi -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
On 5/11/20 6:39 PM, Gabriel Krisman Bertazi wrote: This fourth version of HST applies the suggestion from Mikulas Patocka to do the ktime_get_ns inside the mpath map_bio instead of generic device-mapper code. This means that struct dm_mpath_io gained another 64bit field. For the request-based case, we continue to use the block layer start time information. You should add some info to the multipath.conf.5 man page ( https://git.opensvc.com/gitweb.cgi?p=multipath-tools/.git;a=blob;f=multipath/multipath.conf.5;h=05a5e8ffeb110d969f3b2381eb3b88d7f28380f6;hb=HEAD#l189 ), or none one is going to use it. Thanks. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
Mike Snitzer writes: > OK, that concall's issue had nothing to do with needing higher > resolution time (was about IOPs realized with requested-based vs > bio-based). > > Reality is, DM won't need anything higher resolution than jiffies until > block core's interfaces require something other than jiffies > (e.g. generic_end_io_acct). > > So feel free to proceed with the conditional time fetch solution you > were going to run with (prior to my previous mail asking you to hold > off). > > Sorry for the noise. Thanks, > Mike No problem, thanks for the information. I get started on it. -- Gabriel Krisman Bertazi -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
On Mon, May 11 2020 at 1:31pm -0400, Mike Snitzer wrote: > On Mon, May 11 2020 at 1:11pm -0400, > Gabriel Krisman Bertazi wrote: > > > Mike Snitzer writes: > > > > > On Mon, May 11 2020 at 12:39pm -0400, > > > Gabriel Krisman Bertazi wrote: > > > > > >> Hi, > > >> > > >> This fourth version of HST applies the suggestion from Mikulas Patocka > > >> to do the ktime_get_ns inside the mpath map_bio instead of generic > > >> device-mapper code. This means that struct dm_mpath_io gained another > > >> 64bit field. For the request-based case, we continue to use the block > > >> layer start time information. > > >> > > >> With this modification, I was able obtain similar performance on BIO > > >> to request-based multipath with HST on the benchmarks shared in v1. > > >> > > >> v3: https://www.redhat.com/archives/dm-devel/2020-April/msg00308.html > > >> v2: https://www.redhat.com/archives/dm-devel/2020-April/msg00270.html > > >> v1: https://www.redhat.com/archives/dm-devel/2020-April/msg00176.html > > > > > > I already staged your v3 in linux-next. Please provide an incremental > > > patch that layers on this git branch: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.8 > > > > > > I was hopeful for a flag to be set (e.g. in 'struct path_selector') to > > > reflect whether the path selector expects highres start_time. Makes > > > little sense to incur that extra cost of providing the time if the path > > > selector doesn't even use it. > > > > > > Alternatively, could split out the setting of the time needed by .end_io > > > to a new path_selector_type method (e.g. .set_start_time). And then > > > only use ktime_get_ns() for bio-based if .set_start_time is defined. > > > Would get a little fiddly needing to make sure a stale start_time isn't > > > used... also, makes more sense to conditionally call this > > > .set_start_time just after .start_io is. > > > > Oh, my apologies, I hadn't noticed it was merged. I will make the time > > fetch > > conditional and submit a new patch based on that branch. > > I don't want to waste your time so please don't run with that idea just yet. > > There is a possibility we really _do_ need higher resolution time. > > I'm about to have a concall to discuss some disk IO stat issues with DM > disk stats vs NVMe disk stats (provided by block core). > > I'll let you know the outcome and we can discuss further. OK, that concall's issue had nothing to do with needing higher resolution time (was about IOPs realized with requested-based vs bio-based). Reality is, DM won't need anything higher resolution than jiffies until block core's interfaces require something other than jiffies (e.g. generic_end_io_acct). So feel free to proceed with the conditional time fetch solution you were going to run with (prior to my previous mail asking you to hold off). Sorry for the noise. Thanks, Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
On Mon, May 11 2020 at 1:11pm -0400, Gabriel Krisman Bertazi wrote: > Mike Snitzer writes: > > > On Mon, May 11 2020 at 12:39pm -0400, > > Gabriel Krisman Bertazi wrote: > > > >> Hi, > >> > >> This fourth version of HST applies the suggestion from Mikulas Patocka > >> to do the ktime_get_ns inside the mpath map_bio instead of generic > >> device-mapper code. This means that struct dm_mpath_io gained another > >> 64bit field. For the request-based case, we continue to use the block > >> layer start time information. > >> > >> With this modification, I was able obtain similar performance on BIO > >> to request-based multipath with HST on the benchmarks shared in v1. > >> > >> v3: https://www.redhat.com/archives/dm-devel/2020-April/msg00308.html > >> v2: https://www.redhat.com/archives/dm-devel/2020-April/msg00270.html > >> v1: https://www.redhat.com/archives/dm-devel/2020-April/msg00176.html > > > > I already staged your v3 in linux-next. Please provide an incremental > > patch that layers on this git branch: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.8 > > > > I was hopeful for a flag to be set (e.g. in 'struct path_selector') to > > reflect whether the path selector expects highres start_time. Makes > > little sense to incur that extra cost of providing the time if the path > > selector doesn't even use it. > > > > Alternatively, could split out the setting of the time needed by .end_io > > to a new path_selector_type method (e.g. .set_start_time). And then > > only use ktime_get_ns() for bio-based if .set_start_time is defined. > > Would get a little fiddly needing to make sure a stale start_time isn't > > used... also, makes more sense to conditionally call this > > .set_start_time just after .start_io is. > > Oh, my apologies, I hadn't noticed it was merged. I will make the time fetch > conditional and submit a new patch based on that branch. I don't want to waste your time so please don't run with that idea just yet. There is a possibility we really _do_ need higher resolution time. I'm about to have a concall to discuss some disk IO stat issues with DM disk stats vs NVMe disk stats (provided by block core). I'll let you know the outcome and we can discuss further. Thanks, Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
Mike Snitzer writes: > On Mon, May 11 2020 at 12:39pm -0400, > Gabriel Krisman Bertazi wrote: > >> Hi, >> >> This fourth version of HST applies the suggestion from Mikulas Patocka >> to do the ktime_get_ns inside the mpath map_bio instead of generic >> device-mapper code. This means that struct dm_mpath_io gained another >> 64bit field. For the request-based case, we continue to use the block >> layer start time information. >> >> With this modification, I was able obtain similar performance on BIO >> to request-based multipath with HST on the benchmarks shared in v1. >> >> v3: https://www.redhat.com/archives/dm-devel/2020-April/msg00308.html >> v2: https://www.redhat.com/archives/dm-devel/2020-April/msg00270.html >> v1: https://www.redhat.com/archives/dm-devel/2020-April/msg00176.html > > I already staged your v3 in linux-next. Please provide an incremental > patch that layers on this git branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.8 > > I was hopeful for a flag to be set (e.g. in 'struct path_selector') to > reflect whether the path selector expects highres start_time. Makes > little sense to incur that extra cost of providing the time if the path > selector doesn't even use it. > > Alternatively, could split out the setting of the time needed by .end_io > to a new path_selector_type method (e.g. .set_start_time). And then > only use ktime_get_ns() for bio-based if .set_start_time is defined. > Would get a little fiddly needing to make sure a stale start_time isn't > used... also, makes more sense to conditionally call this > .set_start_time just after .start_io is. Oh, my apologies, I hadn't noticed it was merged. I will make the time fetch conditional and submit a new patch based on that branch. Thanks, -- Gabriel Krisman Bertazi -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
On Mon, May 11 2020 at 12:39pm -0400, Gabriel Krisman Bertazi wrote: > Hi, > > This fourth version of HST applies the suggestion from Mikulas Patocka > to do the ktime_get_ns inside the mpath map_bio instead of generic > device-mapper code. This means that struct dm_mpath_io gained another > 64bit field. For the request-based case, we continue to use the block > layer start time information. > > With this modification, I was able obtain similar performance on BIO > to request-based multipath with HST on the benchmarks shared in v1. > > v3: https://www.redhat.com/archives/dm-devel/2020-April/msg00308.html > v2: https://www.redhat.com/archives/dm-devel/2020-April/msg00270.html > v1: https://www.redhat.com/archives/dm-devel/2020-April/msg00176.html I already staged your v3 in linux-next. Please provide an incremental patch that layers on this git branch: https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.8 I was hopeful for a flag to be set (e.g. in 'struct path_selector') to reflect whether the path selector expects highres start_time. Makes little sense to incur that extra cost of providing the time if the path selector doesn't even use it. Alternatively, could split out the setting of the time needed by .end_io to a new path_selector_type method (e.g. .set_start_time). And then only use ktime_get_ns() for bio-based if .set_start_time is defined. Would get a little fiddly needing to make sure a stale start_time isn't used... also, makes more sense to conditionally call this .set_start_time just after .start_io is. Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v4 0/2] Historical Service Time Path Selector
Hi, This fourth version of HST applies the suggestion from Mikulas Patocka to do the ktime_get_ns inside the mpath map_bio instead of generic device-mapper code. This means that struct dm_mpath_io gained another 64bit field. For the request-based case, we continue to use the block layer start time information. With this modification, I was able obtain similar performance on BIO to request-based multipath with HST on the benchmarks shared in v1. v3: https://www.redhat.com/archives/dm-devel/2020-April/msg00308.html v2: https://www.redhat.com/archives/dm-devel/2020-April/msg00270.html v1: https://www.redhat.com/archives/dm-devel/2020-April/msg00176.html Gabriel Krisman Bertazi (1): md: mpath: Pass IO start time to path selector Khazhismel Kumykov (1): md: mpath: Add Historical Service Time Path Selector drivers/md/Kconfig | 11 + drivers/md/Makefile | 1 + drivers/md/dm-historical-service-time.c | 561 drivers/md/dm-mpath.c | 12 +- drivers/md/dm-path-selector.h | 2 +- drivers/md/dm-queue-length.c| 2 +- drivers/md/dm-service-time.c| 2 +- 7 files changed, 585 insertions(+), 6 deletions(-) create mode 100644 drivers/md/dm-historical-service-time.c -- 2.26.2 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel