public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>,
	linux-scsi@vger.kernel.org, Jens Axboe <jens.axboe@oracle.com>,
	Christoph Hellwig <hch@lst.de>,
	Randy Dunlap <randy.dunlap@oracle.com>
Subject: Update SCSI documentation for 512 byte sector requirement with max_sectors
Date: Thu, 31 Jan 2008 17:58:30 -0800	[thread overview]
Message-ID: <1201831111.11265.192.camel@haakon2.linux-iscsi.org> (raw)
In-Reply-To: <1201808524.3131.46.camel@localhost.localdomain>

On Thu, 2008-01-31 at 13:42 -0600, James Bottomley wrote:
> On Thu, 2008-01-31 at 10:44 -0800, Nicholas A. Bellinger wrote:
> > > In short, and to repeat: almost every internal size counter to block is
> > > in units of 512 byte sectors ... that includes capacity, maximum etc ...
> > > 
> > 
> > Ok, after reading your followup with Geert I see that this looks like a
> > bug in ps3rom.c assuming 2048 byte sectors to calculate .max_sectors
> > (which was originally set to 32 as I mentioned).  Using the setting
> > BOUNCE_SIZE << 9 where BOUNCE_SIZE is the request size in bytes looks
> > like this will solve the issue.  My misunderstanding was
> > that .max_sectors was allowed to be calcuated in non 512 byte sectors,
> > so please disregard my patch.
> > 
> > Geert, .max_sectors for ps3rom.c using 512 byte sectors ends up being
> > 128, yes.?
> > 
> > James, could we put something in the SCSI docs stating that .max_sectors
> > MUST be calculated against 512 byte sectors..?
> 
> If that "we" is royal, then certainly you're welcome to submit a patch
> (just get Randy's ack).  However, do take a look at the existing docs
> first.  Certainly the block layer seems to make this very clear:
> 
> /**
>  * blk_queue_max_sectors - set max sectors for a request for this queue
>  * @q:  the request queue for the device
>  * @max_sectors:  max sectors in the usual 512b unit
>                               ^^^^^^^^^^^^^^^^^^^^^^
>  *
>  * Description:
>  *    Enables a low level driver to set an upper limit on the size of
>  *    received requests.
>  **/
> 

Agreed, here is the patch to make this clear within SCSI.  Randy, does
this look OK..?

Thanks,

--nab

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>

diff --git a/Documentation/scsi/scsi_mid_low_api.txt b/Documentation/scsi/scsi_mid_low_api.txt
index 6f70f2b..570f271 100644
--- a/Documentation/scsi/scsi_mid_low_api.txt
+++ b/Documentation/scsi/scsi_mid_low_api.txt
@@ -1244,13 +1244,12 @@ of interest:
     this_id      - scsi id of host (scsi initiator) or -1 if not known
     sg_tablesize - maximum scatter gather elements allowed by host.
                    0 implies scatter gather not supported by host
-    max_sectors  - maximum number of sectors (usually 512 bytes) allowed
-                   in a single SCSI command. The default value of 0 leads
-                   to a setting of SCSI_DEFAULT_MAX_SECTORS (defined in
-                   scsi_host.h) which is currently set to 1024. So for a
-                   disk the maximum transfer size is 512 KB when max_sectors
-                   is not defined. Note that this size may not be sufficient
-                   for disk firmware uploads.
+    max_sectors  - maximum number of 512 bytes sectors allowed in a single
+                   SCSI command. The default value of 0 leads to a setting
+                   of SCSI_DEFAULT_MAX_SECTORS (defined in scsi_host.h) which
+                   is currently set to 1024. So for a disk the maximum transfer
+                   size is 512 KB when max_sectors is not defined. Note that
+                   this size may not be sufficient for disk firmware uploads.
     cmd_per_lun  - maximum number of commands that can be queued on devices
                    controlled by the host. Overridden by LLD calls to
                    scsi_adjust_queue_depth().
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 5c58d59..84098e3 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -372,7 +372,10 @@ struct scsi_host_template {
        unsigned short sg_tablesize;
 
        /*
-        * If the host adapter has limitations beside segment count
+        * If the host adapter has limitations beside segment count.
+        * Note that this value MUST be calculated in 512 byte sectors,
+        * even if the attached struct scsi_device->sector_size is expected
+        * to use non 512 byte sectors.
         */
        unsigned short max_sectors;



> 


  reply	other threads:[~2008-02-01  1:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <dc3abf350801290318u66474599n2f293832373f3706@mail.gmail.com>
2008-01-31 13:28 ` [Cbe-oss-dev] [PATCH] ps3rom: sector size should be 512 bytes Geert Uytterhoeven
2008-01-31 14:10   ` Nicholas A. Bellinger
2008-01-31 15:34     ` James Bottomley
2008-01-31 16:10       ` Nicholas A. Bellinger
2008-01-31 16:26         ` James Bottomley
2008-01-31 17:28           ` Nicholas A. Bellinger
2008-01-31 17:41             ` Geert Uytterhoeven
2008-01-31 18:06               ` James Bottomley
2008-01-31 18:48                 ` Geert Uytterhoeven
2008-01-31 17:53             ` James Bottomley
2008-01-31 18:44               ` Nicholas A. Bellinger
2008-01-31 19:14                 ` Geert Uytterhoeven
2008-02-01  5:01                   ` Nicholas A. Bellinger
2008-01-31 19:42                 ` James Bottomley
2008-02-01  1:58                   ` Nicholas A. Bellinger [this message]
2008-02-01  2:46                     ` Update SCSI documentation for 512 byte sector requirement with max_sectors Randy Dunlap
2008-02-01  4:38                       ` Nicholas A. Bellinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1201831111.11265.192.camel@haakon2.linux-iscsi.org \
    --to=nab@linux-iscsi.org \
    --cc=Geert.Uytterhoeven@sonycom.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=hch@lst.de \
    --cc=jens.axboe@oracle.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=randy.dunlap@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox