<div dir="ltr">Quick update on this issue-<div>We spoke to Hannes Reinecke and understood that we were using a sub-optimal checker aka 'directio' for testing the paths.</div><div>As recommended to us by Hannes, we tried the 'tur' (Test Unit Ready) instead of the 'directio'.</div><div>Result: We saw huge improvements in the times for the PROUT/PRIN commands.<br>This was the change needed:</div><div><div style="font-size:12.8000001907349px">libmultipath/checkers.h<br>- <font color="#ff0000">#define DEFAULT_CHECKER DIRECTIO</font></div><div style="font-size:12.8000001907349px">+ <font color="#0000ff">#define DEFAULT_CHECKER TUR</font></div></div><div style="font-size:12.8000001907349px"><font color="#0000ff"><br></font></div><div style="font-size:12.8000001907349px"><font color="#000000">However, I still believe that the path_discovery() API should not have the DI_CHECKER bit in the flags passed to it.</font></div><div style="font-size:12.8000001907349px"><font color="#000000">By removing this bit, we saw even better times (about 3x faster) for the PROUT/PRIN commands.</font></div><div style="font-size:12.8000001907349px">Thus, if there are no side-effects/repercussions of removing this bit from the place I mentioned, we could remove the bit IMO.</div><div style="font-size:12.8000001907349px"><br></div><div style="font-size:12.8000001907349px">Thanks,</div><div style="font-size:12.8000001907349px">Satyajit</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 28, 2015 at 12:49 AM, Satyajit Deshmukh <span dir="ltr"><<a href="mailto:satyajit.s.deshmukh@gmail.com" target="_blank">satyajit.s.deshmukh@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hello,<br><br>This is related to an issue raised by Richard Sharpe recently.<br><a href="https://www.redhat.com/archives/dm-devel/2015-August/msg00154.html" target="_blank">https://www.redhat.com/archives/dm-devel/2015-August/msg00154.html</a><br><br>The RPM that was used-<br><a href="http://vault.centos.org/6.6/updates/Source/SPackages/device-mapper-multipath-0.4.9-80.el6_6.3.src.rpm" target="_blank">http://vault.centos.org/6.6/updates/Source/SPackages/device-mapper-multipath-0.4.9-80.el6_6.3.src.rpm</a><br><br><b>Problem:</b><br>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:<br><i>extern int mpath_persistent_reserve_in (int fd, int rq_servact, struct prin_resp *resp, int noisy, int verbose);<br>extern int mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,<br>    unsigned int rq_type, struct prout_param_descriptor *paramp, int noisy,<br>    int verbose);</i><br><br>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.<br><br><b>Root cause of the system wide device scan:</b><br>The system-wide scan is triggered by setting the DI_CHECKER bit in the flags argument to a path_discovery() API:<br> <i> if (path_discovery(pathvec, conf, DI_SYSFS | DI_CHECKER)) {<br>    ret = MPATH_PR_DMMP_ERROR;<br>    goto out1;<br>  }<br><br>  /* get info of all paths from the dm device */<br>  if (get_mpvec (curmp, pathvec, alias)){<br>    condlog(0, "%s: failed to get device info.", alias);<br>    ret = MPATH_PR_DMMP_ERROR;<br>    goto out1;<br>  }  </i><br><br>I believe the DI_CHECKER bit in path_discovery() API is not needed.<br>This is because the path status checking is already done on the specific mpath device in the get_mpvec API.<br><br>The get_mpvec() gets the required struct multipath *mpp, and then invokes<br>pathinfo(pp, conf->hwtable, DI_CHECKER) for each path in the multipath dev.<br>And this checks state of each path for the multipath device.<br> <i> if (mask & DI_CHECKER) {<br>    pp->chkrstate = pp->state = get_state(pp, 0);</i><br><br><b>Possible solution:</b><br>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.<br>I have tested out the following PRIN and PROUT commands and don't see any errors.<br>mpathpersist -i -k /dev/mapper/mpathig<br>mpathpersist -i -r /dev/mapper/mpathig<br>mpathpersist --out --register --param-rk=0A041D3d /dev/mapper/mpathig<br>mpathpersist --out --register --param-sark=0A041D3d /dev/mapper/mpathig<br>mpathpersist --out --reserve --param-rk=0A041D3D --prout-type=5 --verbose=3 /dev/mapper/mpathig<br><br>Request someone to review the patch, suggest changes and guide me in fixing this issue.<br><br>Thanks,<br>Satyajit</div>
</blockquote></div><br></div>