linux-kernel - Re: [PATCH] scsi: Convert scsi_host->cmd_serial_number to odd numbered atomic

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]

Date:	Thu, 11 Nov 2010 09:53:03 -0600
From:	James Bottomley <James.Bottomley@...e.de>
To:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
Cc:	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Jeff Garzik <jeff@...zik.org>, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH] scsi: Convert scsi_host->cmd_serial_number to odd
 numbered atomic_t counter

On Thu, 2010-11-11 at 02:46 -0800, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@...ux-iscsi.org>
> 
> This patch converts struct scsi_host->cmd_serial_number to a 'odd incremented by 2'
> atomic_t counter in scsi_cmd_get_serial(), and moves the host_lock held call in
> jgarzik's DEF_SCSI_QCMD() wrapper back into it's original location in scsi_dispatch_cmd().

Actually, no ... this isn't really a good idea; you're lengthening our
critical path here (an atomic costs a lot more than a simple add under
spinlock).

There are only a few drivers left that actually make use of a serial
number.  Of those, the only modern ones are qla4, lpfc, mpt2sas and
megaraid.

So the next logical step seems to be eliminate the overloading of the
serial number zero value, which removes the last mid-layer use (dpt_i2o
seems to abuse this unnecessarily as well), then the serial number code
can be pushed down into the queuecommand routines of only those drivers
that actually use it.  None of the modern ones seems to have a
legitimate use, so I think their uses can mostly be eliminated.  Thus,
we might be able to get away with a simple queuecommand push down and
never bother with atomics for this (since it's unlikely the legacy users
would convert away from a lock wrapping their queuecommand routines).

James

--
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/