linux-kernel - Re: [PATCH 20/22] atari_scsi: Set a reasonable default for cmd_per

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:	Tue, 15 Mar 2016 07:34:01 -0700
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Finn Thain <fthain@...egraphics.com.au>,
	Hannes Reinecke <hare@...e.de>
Cc:	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Michael Schmitz <schmitzmic@...il.com>,
	linux-m68k@...r.kernel.org, linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Ondrej Zary <linux@...nbow-software.org>,
	Sam Creasey <sammy@...my.net>
Subject: Re: [PATCH 20/22] atari_scsi: Set a reasonable default for
 cmd_per_lun

On Tue, 2016-03-15 at 14:27 +1100, Finn Thain wrote:
> On Mon, 14 Mar 2016, Hannes Reinecke wrote:
> 
> > On 03/14/2016 05:27 AM, Finn Thain wrote:
> > > This setting does not need to be conditional on Atari ST or TT.
> > > 
> > > Without TCQ support, cmd_per_lun == 2 is probably reasonable...
> > > 
> > > Signed-off-by: Finn Thain <fthain@...egraphics.com.au>
> > > 
> > > ---
> > >  drivers/scsi/atari_scsi.c |    3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > Index: linux/drivers/scsi/atari_scsi.c
> > >
> ===================================================================
> > > --- linux.orig/drivers/scsi/atari_scsi.c    2016-03-14
> 15:26:45.000000000 +1100
> > > +++ linux/drivers/scsi/atari_scsi.c 2016-03-14 15:26:55.000000000
> +1100
> > > @@ -750,6 +750,7 @@ static struct scsi_host_template atari_s
> > >     .eh_abort_handler       = atari_scsi_abort,
> > >     .eh_bus_reset_handler   = atari_scsi_bus_reset,
> > >     .this_id                = 7,
> > > +   .cmd_per_lun            = 2,
> > >     .use_clustering         = DISABLE_CLUSTERING,
> > >     .cmd_size               = NCR5380_CMD_SIZE,
> > >  };
> > _2_ ? Are you being overly cheeky here?
> > I sincerely doubt the driver is capable of submitting two
> > simultaneous commands ...
> 
> Right. The LLD has LU busy flags to prevent a LU from being issued 
> more than one command.
> 
> > Care to explain?
> 
> It seemed harmless and it is consistent with the all of the other 
> 5380 drivers.
> 
> I don't know why it was done that way. Perhaps it was done to create 
> a pipeline. That is, to keep a small number of commands in the LLD
> issue queue so that the NCR5380_main() work item does not have to 
> terminate and then get requeued needlessly.

You youngsters nowadays have no grasp of history.

It was done this way so that a fully prepared command was waiting on
the queue rather than having to prepare it in what was then
elv_next_request().  The theory was it was a sort of NAPI because as
soon as the driver called the done() function, block would send us the
next request and we could poke it into the card with the minimum CPU
expenditure (it was all done at softirq level).

Of course, block doesn't function remotely like this today, but
historically that's why most old SCSI drivers set this parameter to one
more than the number of commands they can take.

James