From patchwork Fri May 11 17:56:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lee Duncan X-Patchwork-Id: 10394791 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 97DF560348 for ; Fri, 11 May 2018 17:56:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AE31828F73 for ; Fri, 11 May 2018 17:56:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A1B3028F7A; Fri, 11 May 2018 17:56:54 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E948428F73 for ; Fri, 11 May 2018 17:56:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751457AbeEKR4x (ORCPT ); Fri, 11 May 2018 13:56:53 -0400 Received: from mx2.suse.de ([195.135.220.15]:48080 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750953AbeEKR4w (ORCPT ); Fri, 11 May 2018 13:56:52 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 7DC5BAE4F; Fri, 11 May 2018 17:56:50 +0000 (UTC) From: Lee Duncan To: target-devel@vger.kernel.org Cc: nab@linux-iscsi.org, hare@suse.de, linux-scsi@vger.kernel.org, hch@infradead.org, Lee Duncan Subject: [PATCH v3] target: transport should allow st FM/EOM/ILI reads Date: Fri, 11 May 2018 10:56:24 -0700 Message-Id: <20180511175624.10670-1-lduncan@suse.com> X-Mailer: git-send-email 2.13.6 Sender: target-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: target-devel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When a tape drive is exported via LIO using the pscsi module, a read that requests more bytes per block than the tape can supply returns an empty buffer. This is because the pscsi pass-through target module sees the "ILI" illegal length bit set and thinks there is no reason to return the data. This is a long-standing transport issue, since it assumes that no data need be returned under a check condition, which isn't always the case for tape. Add in a check for tape reads with the ILI, EOM, or FM bits set, with a sense code of NO_SENSE, treating such cases as if the read succeeded. The layered tape driver then "does the right thing" when it gets such a response. Changes from v2: - Cleaned up subject line and bug text formatting - Removed unneeded inner braces - Removed ugly goto - Also updated the "queue full" path to handle this case Changes from RFC: - Moved ugly code from transport to pscsi module - Added checking EOM and FM bits, as well as ILI - fixed malformed patch - Clarified description a bit Signed-off-by: Lee Duncan Reviewed-by: Hannes Reinecke --- drivers/target/target_core_pscsi.c | 23 +++++++++++++++++++- drivers/target/target_core_transport.c | 39 +++++++++++++++++++++++++++++----- include/target/target_core_base.h | 1 + 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index 0d99b242e82e..a9656368a96d 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -689,8 +689,29 @@ static void pscsi_complete_cmd(struct se_cmd *cmd, u8 scsi_status, } after_mode_select: - if (scsi_status == SAM_STAT_CHECK_CONDITION) + if (scsi_status == SAM_STAT_CHECK_CONDITION) { transport_copy_sense_to_cmd(cmd, req_sense); + + /* + * hack to check for TAPE device reads with + * FM/EOM/ILI set, so that we can get data + * back despite framework assumption that a + * check condition means there is no data + */ + if (sd->type == TYPE_TAPE && + cmd->data_direction == DMA_FROM_DEVICE) { + /* + * is sense data valid, fixed format, + * and have FM, EOM, or ILI set? + */ + if (req_sense[0] == 0xf0 && /* valid, fixed format */ + req_sense[2] & 0xe0 && /* FM, EOM, or ILI */ + (req_sense[2] & 0xf) == 0) { /* key==NO_SENSE */ + pr_debug("Tape FM/EOM/ILI status detected. Treat as normal read.\n"); + cmd->se_cmd_flags |= SCF_TREAT_READ_AS_NORMAL; + } + } + } } enum { diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 74b646f165d4..a15a9e3dce11 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2084,12 +2084,24 @@ static void transport_complete_qf(struct se_cmd *cmd) goto queue_status; } - if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) + /* + * Check if we need to send a sense buffer from + * the struct se_cmd in question. We do NOT want + * to take this path of the IO has been marked as + * needing to be treated like a "normal read". This + * is the case if it's a tape read, and either the + * FM, EOM, or ILI bits are set, but there is no + * sense data. + */ + if (!(cmd->se_cmd_flags & SCF_TREAT_READ_AS_NORMAL) && + cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) goto queue_status; switch (cmd->data_direction) { case DMA_FROM_DEVICE: - if (cmd->scsi_status) + /* queue status if not treating this as a normal read */ + if (cmd->scsi_status && + !(cmd->se_cmd_flags & SCF_TREAT_READ_AS_NORMAL)) goto queue_status; trace_target_cmd_complete(cmd); @@ -2194,9 +2206,15 @@ static void target_complete_ok_work(struct work_struct *work) /* * Check if we need to send a sense buffer from - * the struct se_cmd in question. + * the struct se_cmd in question. We do NOT want + * to take this path of the IO has been marked as + * needing to be treated like a "normal read". This + * is the case if it's a tape read, and either the + * FM, EOM, or ILI bits are set, but there is no + * sense data. */ - if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) { + if (!(cmd->se_cmd_flags & SCF_TREAT_READ_AS_NORMAL) && + cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) { WARN_ON(!cmd->scsi_status); ret = transport_send_check_condition_and_sense( cmd, 0, 1); @@ -2238,7 +2256,18 @@ static void target_complete_ok_work(struct work_struct *work) queue_rsp: switch (cmd->data_direction) { case DMA_FROM_DEVICE: - if (cmd->scsi_status) + /* + * if this is a READ-type IO, but SCSI status + * is set, then skip returning data and just + * return the status -- unless this IO is marked + * as needing to be treated as a normal read, + * in which case we want to go ahead and return + * the data. This happens, for example, for tape + * reads with the FM, EOM, or ILI bits set, with + * no sense data. + */ + if (cmd->scsi_status && + !(cmd->se_cmd_flags & SCF_TREAT_READ_AS_NORMAL)) goto queue_status; atomic_long_add(cmd->data_length, diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 9f9f5902af38..922a39f45abc 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -143,6 +143,7 @@ enum se_cmd_flags_table { SCF_ACK_KREF = 0x00400000, SCF_USE_CPUID = 0x00800000, SCF_TASK_ATTR_SET = 0x01000000, + SCF_TREAT_READ_AS_NORMAL = 0x02000000, }; /*