From patchwork Mon Feb 24 19:58:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bart Van Assche X-Patchwork-Id: 3711681 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 16BF49F35F for ; Mon, 24 Feb 2014 19:59:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3A72720136 for ; Mon, 24 Feb 2014 19:59:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5791C2015A for ; Mon, 24 Feb 2014 19:59:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753576AbaBXT7K (ORCPT ); Mon, 24 Feb 2014 14:59:10 -0500 Received: from smtp03.stone-is.org ([87.238.162.65]:37239 "EHLO smtpgw.stone-is.be" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1753529AbaBXT6v (ORCPT ); Mon, 24 Feb 2014 14:58:51 -0500 Received: from localhost (unknown [127.0.0.1]) by smtpgw.stone-is.be (Postfix) with ESMTP id 7E35A335A9D; Mon, 24 Feb 2014 19:58:49 +0000 (UTC) Received: from smtpgw.stone-is.be ([127.0.0.1]) by localhost (smtpgw.stone-is.be [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GWIo6uDbZVYi; Mon, 24 Feb 2014 20:58:48 +0100 (CET) Received: from vz19.stone-is.net (vz19.stone-is.net [87.238.162.57]) by smtpgw.stone-is.be (Postfix) with ESMTP id 649F0335AA4; Mon, 24 Feb 2014 20:58:47 +0100 (CET) X-No-Relay: not in my network X-No-Relay: not in my network X-No-Relay: not in my network X-No-Relay: not in my network X-No-Relay: not in my network X-No-Relay: not in my network Received: from [192.168.1.117] (178-119-65-67.access.telenet.be [178.119.65.67]) by vz19.stone-is.net (Postfix) with ESMTPSA id E0B1B89206; Mon, 24 Feb 2014 20:58:18 +0100 (CET) Message-ID: <530BA476.7040005@acm.org> Date: Mon, 24 Feb 2014 20:58:46 +0100 From: Bart Van Assche User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: David Dillow CC: Roland Dreier , Sagi Grimberg , Vu Pham , Sebastian Riemer , linux-rdma Subject: Re: [PATCH 3/6] IB/srp: Fail SCSI commands silently References: <5305DE01.7030004@acm.org> <5305DE67.9070805@acm.org> <1392954937.26557.3.camel@haswell.thedillows.org> <53071B2E.2050302@acm.org> <1393047697.12377.11.camel@haswell.thedillows.org> In-Reply-To: <1393047697.12377.11.camel@haswell.thedillows.org> X-Enigmail-Version: 1.6 Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 02/22/14 06:41, David Dillow wrote: > I didn't suggest that -- I'm saying add a common functionality to turn > on/off the message printing for commands that failed due to a dead > transport. If it is useful for SRP initiators, it is probably useful for > SAS and iSCSI imitators as well. > > At a quick look, it seems you need to add a new value to the > rq_flag_bits enum in include/linux/blk_types.h, __REQ_FAILED_TRANSPORT, > somewhere before __REQ_NR_BITS. And a #define in the style of those > following it. Use that flag instead of REQ_QUIET in the patch we are > discussing, and change the code in scsi_lib.c to check > > ((req->cmd_flags & REQ_FAILED_TRANSPORT) && $user_wants_this_knob) || !(req->cmd_flags & REQ_QUIET) > > before calling scsi_print_sense(), with appropriate naming, of course. > This gives you a central place to control it, and allows for consistency > between initiators. > > I agree it is much easier to just fix it in SRP, especially given the > glacial rate of change for the SCSI mid-layer, but I really think a > central control and driver-by-driver enablement would be the best > approach. YMMV. Hello Dave, That makes sense to me. Do you think the (untested) patch below could be a valid alternative to the above proposal ? --- drivers/scsi/scsi_lib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 7bd7f0d..124ab53 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -857,7 +857,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) */ if ((sshdr.asc == 0x0) && (sshdr.ascq == 0x1d)) ; - else if (!(req->cmd_flags & REQ_QUIET)) + else if (cmd->device->sdev_state != SDEV_TRANSPORT_OFFLINE && + !(req->cmd_flags & REQ_QUIET)) scsi_print_sense("", cmd); result = 0; /* BLOCK_PC may have set error */