2010/12/13 Jeff Layton <jlayton@xxxxxxxxxx>:
> On Sun, 12 Dec 2010 17:52:26 -0600
> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>
>> On Fri, Dec 10, 2010 at 9:44 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>> > Make it use a switch statement based on the value of the midStatus. If
>> > the resp_buf is set, then MID_RESPONSE_RECEIVED is too.
>> >
>> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> > ---
>> > fs/cifs/transport.c | 36 ++++++++++++++++++------------------
>> > 1 files changed, 18 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
>> > index 0c0dadd..05ced17 100644
>> > --- a/fs/cifs/transport.c
>> > +++ b/fs/cifs/transport.c
>> > @@ -356,33 +356,33 @@ handle_mid_result(struct mid_q_entry *mid, struct TCP_Server_Info *server)
>> > {
>> > int rc = 0;
>> >
>> > - spin_lock(&GlobalMid_Lock);
>> > + cFYI(1, "%s: cmd=%d mid=%d state=%d", __func__, mid->command,
>> > + mid->mid, mid->midState);
>> >
>> > - if (mid->resp_buf) {
>> > + spin_lock(&GlobalMid_Lock);
>> > + switch (mid->midState) {
>> > + case MID_RESPONSE_RECEIVED:
>> > spin_unlock(&GlobalMid_Lock);
>> > return rc;
>> > - }
>> > -
>> > - cERROR(1, "No response to cmd %d mid %d", mid->command, mid->mid);
>> > - if (mid->midState == MID_REQUEST_SUBMITTED) {
>> > - if (server->tcpStatus == CifsExiting)
>> > + case MID_REQUEST_SUBMITTED:
>> > + /* socket is going down, reject all calls */
>> > + if (server->tcpStatus == CifsExiting) {
>> > + cERROR(1, "%s: canceling mid=%d cmd=0x%x state=%d",
>> > + __func__, mid->mid, mid->command, mid->midState);
>> > rc = -EHOSTDOWN;
>> > - else
>> > - mid->midState = MID_RETRY_NEEDED;
>> > - }
>> > -
>> > - if (rc != -EHOSTDOWN) {
>> > - if (mid->midState == MID_RETRY_NEEDED) {
>> > - rc = -EAGAIN;
>> > - cFYI(1, "marking request for retry");
>> > - } else {
>> > - rc = -EIO;
>> > + break;
>> > }
>> > + mid->midState = MID_RETRY_NEEDED;
>>
>> Would it be cleaner to set rc to -EAGAIN instead of setting state here
>> to fall down and then setting rc as -EAGAIN?
>>
>
> I'm not sure it'll be any more clear. It seems clear enough to me -- if
> it's still SUBMITTED, then we want to treat it if it were
> "RETRY_NEEDED".
>
> Honestly, this really shouldn't happen. With this patchset, you should
> never reach this function if the state is still 'SUBMITTED'. I left it
> there for completeness sake and future-proofness.
>
>> > + case MID_RETRY_NEEDED:
>> > + rc = -EAGAIN;
>> > + break;
>> > + default:
>> > + cERROR(1, "%s: invalid mid state mid=%d state=%d", __func__,
>> > + mid->mid, mid->midState);
>> > }
>> > spin_unlock(&GlobalMid_Lock);
>> >
>> > DeleteMidQEntry(mid);
>> > - /* Update # of requests on wire to server */
>> > atomic_dec(&server->inFlight);
>> > wake_up(&server->request_q);
>> >
>> > --
>> > 1.7.3.2
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> > the body of a message to majordomo@xxxxxxxxxxxxxxx
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >
>
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Reviewed-by: Pavel Shilovsky <piastryyy@xxxxxxxxx>
--
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
