[nvmewin] ProcessIO Function

[Luse, Paul E]

Yes, that's what I meant below in my last sentence, we need to complete to storport in both of those cases and use SRB_STATUS_ERROR for those cases.  If you want to implement this that'd be great (maybe just have one storport completion at the bottom of the function and set the error in earlier places; to avoid goto please use try/finally in this routine.  Or if you want to focus on the formatNVM I'll clean this up as well as check the callers for mis-use of procession failures.

Thx
Paul

From: Chang, Alex [mailto:Alex.Chang at idt.com]
Sent: Thursday, January 19, 2012 12:58 PM
To: Luse, Paul E; nvmewin at lists.openfabrics.org
Subject: RE: ProcessIO Function

Yes, I'd agree make it as simple as possible. However, there are two places in ProcessIo returning FALSE without calling StorPortNotification to complete the request:
Failure on calling storportGetCurrentProcessorNumber.
Failure on calling NVMeMapCore2Queue.
We should complete the request when either one happens?

Thanks,
Alex

________________________________
From: Luse, Paul E [mailto:paul.e.luse at intel.com]<mailto:[mailto:paul.e.luse at intel.com]>
Sent: Thursday, January 19, 2012 11:46 AM
To: Chang, Alex; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: RE: ProcessIO Function
Ahh, OK thanks.  I'd prefer we simply have it by rule:


-         If process IO fails then the IO is complete to storport (if it had an SRB) with the appropriate status

-         If process IO succeeds then the IO is in flight

If we add this flag then we also have to change the return value for process IO so the caller know why it failed so it can return the IO, if needed to storport.  No point in having every caller implement code for 'return status busy' when it can be done in one central location where the error happens. Make sense?

Thx
Paul


From: Chang, Alex [mailto:Alex.Chang at idt.com]<mailto:[mailto:Alex.Chang at idt.com]>
Sent: Thursday, January 19, 2012 12:27 PM
To: Luse, Paul E; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: RE: ProcessIO Function

Hi Paul,

Sorry, I did not mean init state machine. It's the new Format NVM state machine. After Format NVM command completes successfully, Identify commands are being issued to re-fetch the structures. We all call ProcessIo to issue the commands and its return type is BOOLEAN, which only indicates whether the command had been issued or not. In some error cases, such as failure on getting CmdID, ProcessIo calls StorPortNotification to complete the request and callers have no way to tell if the request had been completed or not. To avoid complete the request twice, I'd like to suggest adding one extra parameter in ProcessIO like:

BOOLEAN
ProcessIo(
    __in PNVME_DEVICE_EXTENSION pAdapterExtension,
    __in PNVME_SRB_EXTENSION pSrbExtension,
    __in NVME_QUEUE_TYPE QueueType,
    __out RequestCompleted
)

Thanks,

Alex

________________________________
From: Luse, Paul E [mailto:paul.e.luse at intel.com]<mailto:[mailto:paul.e.luse at intel.com]>
Sent: Thursday, January 19, 2012 11:06 AM
To: Chang, Alex; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: RE: ProcessIO Function
Hi Alex-

I'm not totally sure I understand what you are trying to say but I think I do.  What does "In SNTI codes" mean?  I assume you're saying that there's some inconsistency with what some callers are assuming about the return value from ProcessIO() and, if so, good catch!  Clearly we cannot complete the same IO twice J  Haven't seen your formatNVM code yet but if any of the sub-commands you need to issue as art of handling formatNVM fails then you should fail the whole command gracefully.  Also, how does any of this relate to init state machine?

We may not have time to talk about this in the meeting so if you could reply and identify specific cases where you see callers of ProcessIO doing the wrong thing and propose a fix that would be great.  I don't believe we want the caller to have to complete the IO if processIO fails, procession should do that however just looking at the code doesn't consistently do that.  We should scrub it so that callers know that if procession fails that the IO has been completed and if it completes the IO has been issued.  We should also fix procession to complete all failures with appropriate error codes.

Thanks
Paul

From: Chang, Alex [mailto:Alex.Chang at idt.com]<mailto:[mailto:Alex.Chang at idt.com]>
Sent: Thursday, January 19, 2012 11:40 AM
To: Luse, Paul E; nvmewin at lists.openfabrics.org<mailto:nvmewin at lists.openfabrics.org>
Subject: ProcessIO Function

Hi all

I tested Format NVM last night and it basically works fine. However, when I re-visited the state machine, there is potential problem related to ProcessIo function, which completes the request back to Storport in error cases. The return value of ProcessIo only indicates whether the command had been issued to controller or not. It does NOT tell if it completes the request. In SNTI codes, Ray re-uses the Srb Extension to issue command(s). If ProcessIO fails, ASSERTION is called. Should I do the same if it fails when re-fetching Identify structures after Format NVM succeeds? Question is we don't want to complete the same request twice. We should talk about this later.

Thanks,
Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/nvmewin/attachments/20120119/d5798a7e/attachment.html>