linux-kernel - Re: [PATCH 03/12] libsas: Convert to host_lock less w/ interrupts disabled externally

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4D0F1A99.8000703@panasas.com>
Date:	Mon, 20 Dec 2010 10:58:01 +0200
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	linux-iscsi-target-dev@...glegroups.com
CC:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	James Bottomley <James.Bottomley@...e.de>,
	Jeff Garzik <jeff@...zik.org>, Christoph Hellwig <hch@....de>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Hannes Reinecke <hare@...e.de>,
	Mike Christie <michaelc@...wisc.edu>,
	Mike Anderson <andmike@...ux.vnet.ibm.com>,
	Tejun Heo <tj@...nel.org>, Vasu Dev <vasu.dev@...ux.intel.com>,
	Tim Chen <tim.c.chen@...ux.intel.com>,
	Andi Kleen <ak@...ux.intel.com>,
	Ravi Anand <ravi.anand@...gic.com>,
	Andrew Vasquez <andrew.vasquez@...gic.com>,
	Joe Eykholt <jeykholt@...co.com>,
	James Smart <james.smart@...lex.com>,
	Douglas Gilbert <dgilbert@...erlog.com>,
	adam radford <aradford@...il.com>,
	Kashyap Desai <Kashyap.Desai@....com>,
	MPTFusionLinux <DL-MPTFusionLinux@....com>
Subject: Re: [PATCH 03/12] libsas: Convert to host_lock less w/ interrupts
 disabled externally

On 12/19/2010 11:21 PM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@...ux-iscsi.org>
> 
> This patch converts the libsas queuecommand to run in host_lock less mode
> w/ the new IRQ_DISABLE_SCSI_QCMD() that disables interrupts while calling
> ->queuecommand() dispatch.
> 
> Signed-off-by: Nicholas A. Bellinger <nab@...ux-iscsi.org>
> ---
>  drivers/scsi/libsas/sas_scsi_host.c |   12 ++----------
>  1 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
> index 29251fa..011580f 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -185,24 +185,17 @@ int sas_queue_up(struct sas_task *task)
>  /**
>   * sas_queuecommand -- Enqueue a command for processing
>   * @parameters: See SCSI Core documentation
> - *
> - * Note: XXX: Remove the host unlock/lock pair when SCSI Core can
> - * call us without holding an IRQ spinlock...
>   */
> -static int sas_queuecommand_lck(struct scsi_cmnd *cmd,
> +static int sas_queuecommand_irq_disable(struct scsi_cmnd *cmd,
>  		     void (*scsi_done)(struct scsi_cmnd *))
> -	__releases(host->host_lock)
>  	__acquires(dev->sata_dev.ap->lock)
>  	__releases(dev->sata_dev.ap->lock)
> -	__acquires(host->host_lock)
>  {
>  	int res = 0;
>  	struct domain_device *dev = cmd_to_domain_dev(cmd);
>  	struct Scsi_Host *host = cmd->device->host;
>  	struct sas_internal *i = to_sas_internal(host->transportt);
>  
> -	spin_unlock_irq(host->host_lock);
> -
>  	{
>  		struct sas_ha_struct *sas_ha = dev->port->ha;
>  		struct sas_task *task;
> @@ -250,11 +243,10 @@ static int sas_queuecommand_lck(struct scsi_cmnd *cmd,
>  		}
>  	}
>  out:
> -	spin_lock_irq(host->host_lock);
>  	return res;
>  }
>  
> -DEF_SCSI_QCMD(sas_queuecommand)
> +IRQ_DISABLE_SCSI_QCMD(sas_queuecommand)
>  

I hate this new macro. It is so simple by now. And anyway you are
doing them one by one and auditing the code. Please completely drop
this macro and open code it. There is no "safety" argument to this
ugliness, any more.

(Completely drop the [PATCH 02/12] scsi: Add IRQ_DISABLE_SCSI_QCMD wrapper
 patch and redo the users)

Thanks
Boaz


>  static void sas_eh_finish_cmd(struct scsi_cmnd *cmd)
>  {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/