<div dir="ltr">Hi Alex,<div><br></div><div>These changes look good. we approve the patch.</div><div><br></div><div>Thanks</div><div>Parag Sheth</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 17, 2014 at 8:21 AM, Alex Chang <span dir="ltr"><<a href="mailto:Alex.Chang@pmcs.com" target="_blank">Alex.Chang@pmcs.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<u></u><u></u>
<div lang="EN-US" link="blue" vlink="purple" style="margin-left:7.5pt;margin-top:7.5pt;margin-right:7.5pt;margin-bottom:7.5pt">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Dear all,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Please review/test the revised patch and provide your feedback. I will start to collect approvals on Thursday.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Alex<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> SUMAN PRAKASH B [mailto:<a href="mailto:suman.p@samsung.com" target="_blank">suman.p@samsung.com</a>]
<br>
<b>Sent:</b> Monday, November 17, 2014 1:17 AM<br>
<b>To:</b> <a href="mailto:nvmewin@lists.openfabrics.org" target="_blank">nvmewin@lists.openfabrics.org</a>; Alex Chang; <a href="mailto:parag.sheth@seagate.com" target="_blank">parag.sheth@seagate.com</a>; <a href="mailto:judy.brock@ssi.samsung.com" target="_blank">judy.brock@ssi.samsung.com</a><br>
<b>Subject:</b> RE: Re: [nvmewin] Samsung patch for Hot plug fixes<u></u><u></u></span></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<pre>Content-Type: text/plain; charset=UTF-8<u></u><u></u></pre>
<pre>Content-Transfer-Encoding: 8bit<u></u><u></u></pre>
<pre>Date: %%SENT_DATE%%<u></u><u></u></pre>
<pre>Subject: Suspect Message Quarantined <u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<pre><u></u> <u></u></pre>
<pre><u></u> <u></u></pre>
<pre>WARNING: The virus scanner was unable to scan an attachment in an email message sent to you. This attachment could possibly contain viruses or other malicious programs. The attachment could not be scanned for the following reasons:<u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<pre>%%DESC%%<u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<pre>The full message and the attachment have been stored in the quarantine.<u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<pre>The identifier for this message is '%%QID%%'.<u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<pre>Access the quarantine at: <u></u><u></u></pre>
<pre><a href="https://puremessage.pmc-sierra.bc.ca:28443/" target="_blank">https://puremessage.pmc-sierra.bc.ca:28443/</a><u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<pre>For more information on PMC's Anti-Spam system:<u></u><u></u></pre>
<pre><a href="http://pmc-intranet/wiki/index.php/Outlook:Anti-Spam_FAQ" target="_blank">http://pmc-intranet/wiki/index.php/Outlook:Anti-Spam_FAQ</a><u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<pre>IT Services<u></u><u></u></pre>
<pre>PureMessage Admin <u></u><u></u></pre>
<pre><u></u> <u></u></pre>
<p><span style="font-family:"Calibri","sans-serif";color:#1f497d">Hi Alex,</span><u></u><u></u></p>
<p> <u></u><u></u></p>
<p><span style="font-family:"Calibri","sans-serif";color:#1f497d">Please find attached the revised code with the following review comments incorporated. Password is
</span><strong><span style="font-family:"Calibri","sans-serif";color:blue">samsung123</span></strong><span style="font-family:"Calibri","sans-serif";color:#1f497d">.</span><u></u><u></u></p>
<p> <u></u><u></u></p>
<p>1. In function NVMeWaitForCtrlRDY(), you return TRUE when device is removed. And as per your explanation, this is to avoid delay in removing device from device manager. But returning TRUE is not intuitive. This function actually failed and hence we should
be returning FALSE. And if this breaks your flow than the max delay would be 500 milliseconds. I would say that is negligible from user display point of view.<u></u><u></u></p>
<p><span style="font-family:"Calibri","sans-serif";color:#1f497d">a. Moved the pAE->ShutdownInProgress inside the else part in NVMeAdapterControlPowerDown().<br>
b. Returned FALSE from NVMeWaitForCtrlRDY() when device is removed.</span><span style="font-size:9.0pt;font-family:"Arial","sans-serif""><u></u><u></u></span></p>
<p> <u></u><u></u></p>
<p><span style="color:black">2. Function brief needs to be updated for NVMeDetectPendingCmds() for the 3rd parameter.</span><span style="font-family:"Calibri","sans-serif";color:#1f497d"><br>
Function brief updated for NVMeDetectPendingCmds() for the 3rd parameter.</span><u></u><u></u></p>
<p> <u></u><u></u></p>
<p><span style="font-family:"Calibri","sans-serif";color:#1f497d">Thanks,</span><u></u><u></u></p>
<p><span style="font-family:"Calibri","sans-serif";color:#1f497d">Suman</span><u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Arial","sans-serif""><u></u> <u></u></span></p>
<p>------- <b>Original Message</b> -------<span style="font-size:9.0pt;font-family:"Arial","sans-serif""><u></u><u></u></span></p>
<p><b>Sender</b> : Alex Chang<<a href="mailto:Alex.Chang@pmcs.com" target="_blank">Alex.Chang@pmcs.com</a>><u></u><u></u></p>
<p><b>Date</b> : Nov 15, 2014 03:36 (GMT+05:00)<u></u><u></u></p>
<p><b>Title</b> : RE: Re: [nvmewin] Samsung patch for Hot plug fixes<u></u><u></u></p>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Suman,<u></u><u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Could you please revise the codes, test and send it out at your earliest convenience?<u></u><u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thank you!<u></u><u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Alex<u></u><u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Parag Sheth [mailto:<a href="mailto:parag.sheth@seagate.com" target="_blank">parag.sheth@seagate.com</a>]
<br>
<b>Sent:</b> Friday, November 14, 2014 1:56 PM<br>
<b>To:</b> <a href="mailto:suman.p@samsung.com" target="_blank">suman.p@samsung.com</a><br>
<b>Cc:</b> Alex Chang; <a href="mailto:nvmewin@lists.openfabrics.org" target="_blank">nvmewin@lists.openfabrics.org</a>; <a href="mailto:judy.brock@ssi.samsung.com" target="_blank">judy.brock@ssi.samsung.com</a><br>
<b>Subject:</b> Re: Re: [nvmewin] Samsung patch for Hot plug fixes<u></u><u></u></span></p>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><u></u> <u></u></p>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Hi Suman,<u></u><u></u></p>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><u></u> <u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">As long as this change passes all your test cases - I am ok with that.<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><u></u> <u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Thanks<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Parag Sheth<u></u><u></u></p>
</div>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><u></u> <u></u></p>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">On Fri, Nov 14, 2014 at 12:40 AM, SUMAN PRAKASH B <<a href="mailto:suman.p@samsung.com" target="_blank">suman.p@samsung.com</a>> wrote:<u></u><u></u></p>
<div>
<p>Hi Parag,<u></u><u></u></p>
<p> <u></u><u></u></p>
<p>Thanks for your feedback. Please find my comments inline.<u></u><u></u></p>
<p> <u></u><u></u></p>
<p>Thanks,<u></u><u></u></p>
<p>Suman<u></u><u></u></p>
<p> <u></u><u></u></p>
<p>------- <b>Original Message</b> -------<u></u><u></u></p>
<p><b>Sender</b> : Parag Sheth<<a href="mailto:parag.sheth@seagate.com" target="_blank">parag.sheth@seagate.com</a>><u></u><u></u></p>
<p><b>Date</b> : Nov 14, 2014 06:31 (GMT+05:00)<u></u><u></u></p>
<p><b>Title</b> : Re: [nvmewin] Samsung patch for Hot plug fixes<u></u><u></u></p>
<p> <u></u><u></u></p>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Hi Suman, <u></u><u></u></p>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><u></u> <u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Here are my observations<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><u></u> <u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">1. In function NVMeWaitForCtrlRDY(), you return TRUE when device is removed. And as per your explanation, this is to avoid delay in removing device from device manager. But returning TRUE is not intuitive.
This function actually failed and hence we should be returning FALSE. And if this breaks your flow than the max delay would be 500 milliseconds. I would say that is negligible from user display point of view.<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><strong><span style="color:blue">[Suman]
</span></strong><span style="color:blue">Agreed. We initially returned FALSE from NVMeWaitForCtrlRDY() when device is removed. We changed it to TRUE for the following reason.</span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="color:blue">When device is removed, the NVMeBuildIo() -> SRB_FUNCTION_PNP -> NVMeAdapterControlPowerDown() -> NVMeResetAdapter() -> NVMeWaitForCtrlRDY() was returning FALSE because of which the
pAE->ShutdownInProgess was not set to TRUE in NVMeAdaptercontrolPowerDown. <br>
After this when NVMeAdapterControl() -> ScsiStopAdapter -> NVMeAdapterControlPowerDown() is invoked, since the pAE->ShutdownInProgess was not set to TRUE in NVMeBuildIo(), the NVMeDetectPendingCmds() and NVMeResetAdapter() is executed again. This should not
be executed second time.<br>
To avoid this we retured TRUE from NVMeWaitForCtrlRDY() so that pAE->ShutdownInProgress is set to TRUE.</span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"> <u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="color:blue">To resolve this, in NVMeAdapterControlPowerDown(), we can move setting the pAE->ShutdownInProgress inside the else part and return FALSE when device is removed from NVMeWaitForCtrlRDY()
as follows.</span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="color:blue">NVMeAdapterControlPowerDown()<br>
{<br>
...<br>
if (pAE->ShutdownInProgress == TRUE) {<br>
/* Shutdown */<br>
status = TRUE;<br>
} else {<br>
<span style="background:yellow">pAE->ShutdownInProgress = TRUE;</span></span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="color:blue"> /* Hibernate or Sleep - sanity check that there is no cmd pending */<br>
if (NVMeDetectPendingCmds(pAE, FALSE, SRB_STATUS_BUS_RESET) == TRUE)<br>
return status;</span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="color:blue"> /* Stop the controller, but do not free the resources */<br>
if (NVMeResetAdapter(pAE) != TRUE) {<br>
return (FALSE);<br>
}<br>
}<br>
...<br>
}</span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><span style="color:blue">Kindly let us know your opinion.</span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><br>
<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">2. Function brief needs to be updated for NVMeDetectPendingCmds() for the 3rd parameter.<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><strong><span style="color:blue">[Suman]
</span></strong><span style="color:blue">Yes. We will change this.</span><u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><br>
<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Other than these 2, your changes look good.<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt"><u></u> <u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Thanks<u></u><u></u></p>
</div>
<div>
<p style="margin-bottom:0in;margin-bottom:.0001pt">Parag Sheth<u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
<p> <u></u><u></u></p>
<table border="0" cellpadding="0">
<tbody>
<tr>
<td style="padding:.75pt .75pt .75pt .75pt">
<p><img border="0" width="520" height="144" src="cid:image001.gif@01D0023F.70A856A0"><u></u><u></u></p>
</td>
</tr>
</tbody>
</table>
<p class="MsoNormal"><span style="border:solid windowtext 1.0pt;padding:0in"><img border="0" width="100" height="100" src="cid:~WRD359.jpg" alt="Image removed by sender."></span><u></u><u></u></p>
</div>
</div>
</blockquote></div><br></div>