[dm-devel] Unnecessary scanning of all SCSI devices in mpath_persistent_reserve_in/out command, sent to a specific mpath device,

[Satyajit Deshmukh]

Quick update on this issue-
We spoke to Hannes Reinecke and understood that we were using a sub-optimal
checker aka 'directio' for testing the paths.
As recommended to us by Hannes, we tried the 'tur' (Test Unit Ready)
instead of the 'directio'.
Result: We saw huge improvements in the times for the PROUT/PRIN commands.
This was the change needed:
libmultipath/checkers.h
- #define DEFAULT_CHECKER DIRECTIO
+ #define DEFAULT_CHECKER TUR

However, I still believe that the path_discovery() API should not have the
DI_CHECKER bit in the flags passed to it.
By removing this bit, we saw even better times (about 3x faster) for the
PROUT/PRIN commands.
Thus, if there are no side-effects/repercussions of removing this bit from
the place I mentioned, we could remove the bit IMO.

Thanks,
Satyajit

On Fri, Aug 28, 2015 at 12:49 AM, Satyajit Deshmukh <
satyajit.s.deshmukh at gmail.com> wrote:

> Hello,
>
> This is related to an issue raised by Richard Sharpe recently.
> https://www.redhat.com/archives/dm-devel/2015-August/msg00154.html
>
> The RPM that was used-
>
> http://vault.centos.org/6.6/updates/Source/SPackages/device-mapper-multipath-0.4.9-80.el6_6.3.src.rpm
>
> *Problem:*
> From reading the source code, I understand that 2 APIs defined in
> libmpathpersist/mpath_persist.c do a system-wide scan of all SCSI devices
> by sending asynchronous IO (io_submit calls) to test if SCSI device path is
> good:
>
>
>
> *extern int mpath_persistent_reserve_in (int fd, int rq_servact, struct
> prin_resp *resp, int noisy, int verbose);extern int
> mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
> unsigned int rq_type, struct prout_param_descriptor *paramp, int noisy,
> int verbose);*
>
> So, to get a persistent reservation on a single /dev/mapper/mpathx device
> on a system having 400 SCSI devices, current code does 400 reads of 4K
> each.(512 bytes of 8 sectors), leading to huge delays.
>
> *Root cause of the system wide device scan:*
> The system-wide scan is triggered by setting the DI_CHECKER bit in the
> flags argument to a path_discovery() API:
>
>
>
>
>
>
>
>
>
>
> * if (path_discovery(pathvec, conf, DI_SYSFS | DI_CHECKER)) {    ret =
> MPATH_PR_DMMP_ERROR;    goto out1;  }  /* get info of all paths from the dm
> device */  if (get_mpvec (curmp, pathvec, alias)){    condlog(0, "%s:
> failed to get device info.", alias);    ret = MPATH_PR_DMMP_ERROR;    goto
> out1;  }  *
>
> I believe the DI_CHECKER bit in path_discovery() API is not needed.
> This is because the path status checking is already done on the specific
> mpath device in the get_mpvec API.
>
> The get_mpvec() gets the required struct multipath *mpp, and then invokes
> pathinfo(pp, conf->hwtable, DI_CHECKER) for each path in the multipath dev.
> And this checks state of each path for the multipath device.
>
> * if (mask & DI_CHECKER) {    pp->chkrstate = pp->state = get_state(pp,
> 0);*
>
> *Possible solution:*
> I have a patch ready that simply removes the DI_CHECKER bit from the
> path_discovery() calls inside mpath_persistent_reserve_in() and
> mpath_persistent_reserve_out(). Attached the patch.
> I have tested out the following PRIN and PROUT commands and don't see any
> errors.
> mpathpersist -i -k /dev/mapper/mpathig
> mpathpersist -i -r /dev/mapper/mpathig
> mpathpersist --out --register --param-rk=0A041D3d /dev/mapper/mpathig
> mpathpersist --out --register --param-sark=0A041D3d /dev/mapper/mpathig
> mpathpersist --out --reserve --param-rk=0A041D3D --prout-type=5
> --verbose=3 /dev/mapper/mpathig
>
> Request someone to review the patch, suggest changes and guide me in
> fixing this issue.
>
> Thanks,
> Satyajit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20150831/86d2753d/attachment.htm>