From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759407Ab0I0OTc (ORCPT ); Mon, 27 Sep 2010 10:19:32 -0400 Received: from mtagate7.uk.ibm.com ([194.196.100.167]:33628 "EHLO mtagate7.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759023Ab0I0OTa (ORCPT ); Mon, 27 Sep 2010 10:19:30 -0400 Date: Mon, 27 Sep 2010 16:19:23 +0200 From: Christof Schmitt To: "Nicholas A. Bellinger" Cc: Brian King , linux-scsi , linux-kernel , Vasu Dev , Tim Chen , Andi Kleen , Matthew Wilcox , James Bottomley , Mike Christie , James Smart , Andrew Vasquez , FUJITA Tomonori , Hannes Reinecke , Joe Eykholt , Christoph Hellwig , MPTFusionLinux , "eata.c maintainer" Subject: Re: [RFC v3 01/15] scsi: Drop struct Scsi_Host->host_lock usage in scsi_dispatch_cmd() Message-ID: <20100927141923.GC8473@schmichrtp.mainz.de.ibm.com> References: <1285285052-16351-1-git-send-email-nab@linux-iscsi.org> <4C9CAA80.1030702@linux.vnet.ibm.com> <1285361040.1849.235.camel@haakon2.linux-iscsi.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1285361040.1849.235.camel@haakon2.linux-iscsi.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 24, 2010 at 01:44:00PM -0700, Nicholas A. Bellinger wrote: > On Fri, 2010-09-24 at 08:41 -0500, Brian King wrote: > > On 09/23/2010 06:37 PM, Nicholas A. Bellinger wrote: > > > @@ -651,7 +655,6 @@ static inline void scsi_cmd_get_serial(struct Scsi_Host *host, struct scsi_cmnd > > > int scsi_dispatch_cmd(struct scsi_cmnd *cmd) > > > { > > > struct Scsi_Host *host = cmd->device->host; > > > - unsigned long flags = 0; > > > unsigned long timeout; > > > int rtn = 0; > > > > > > @@ -736,15 +739,11 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd) > > > scsi_done(cmd); > > > goto out; > > > } > > > - > > > - spin_lock_irqsave(host->host_lock, flags); > > > /* > > > - * AK: unlikely race here: for some reason the timer could > > > - * expire before the serial number is set up below. > > > - * > > > - * TODO: kill serial or move to blk layer > > > + * Note that scsi_cmd_get_serial() used to be called here, but > > > + * now we expect the legacy SCSI LLDs that actually need this > > > + * to call it directly within their SHT->queuecommand() caller. > > > */ > > > - scsi_cmd_get_serial(host, cmd); > > > > > > if (unlikely(host->shost_state == SHOST_DEL)) { > > > cmd->result = (DID_NO_CONNECT << 16); > > > @@ -753,7 +752,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd) > > > trace_scsi_dispatch_cmd_start(cmd); > > > rtn = host->hostt->queuecommand(cmd, scsi_done); > > > } > > > - spin_unlock_irqrestore(host->host_lock, flags); > > > + > > > if (rtn) { > > > trace_scsi_dispatch_cmd_error(cmd, rtn); > > > if (rtn != SCSI_MLQUEUE_DEVICE_BUSY && > > > > Are you planning a future revision that moves the acquiring of the host lock > > into the LLDD's queuecommand for all the other drivers you don't currently > > touch in this patch set? > > > > Hi Brian, > > I was under the impression that this would be unnecessary for the vast > majority of existing LLD drivers, but if you are aware of specific LLDs > that would still need the struct Scsi_Host->host_lock held in their > SHT->queuecommand() for whaterver reason please let me know and I would > be happy to include this into an RFCv4. > > Thanks for your comments! zfcp relies on having the interrupts disabled when calling queuecommand. Without the spin_lock_irqsave in scsi_dispatch_cmd, the locking in zfcp_fsf_send_fcp_command_task has to be changed from spin_lock(&qdio->req_q_lock) to spin_lock_irqsave. It is a simple change, but other drivers might have similar requirements. Christof