Warning: about not setting max_sectors

Warning: about not setting max_sectors

[James Bottomley]

For some reason, the SCSI core is now warning about drivers which don't set 
max_sectors.

Just for the record, we have two parameters in the template:

sg_tablesize which determines the maximum number of scatter gather entries the 
host can have (corresponds to max_phys_segments in the block layer), and

max_sectors, which imposes a maximum overall transfer length (corresponds to 
max_sectors in the block queue).

The comments over max_sectors list it as optional (if unset, we set it to the 
scsi equivalent of machine infinity---well, actually 1024, or 512Mb for usual 
blocks).

The reason it's unset on so many drivers is that usually, they have no 
absolute transfer limit, so they're just bounded by the number of entries in 
the scatter-gather list.

I can't see a reason to force every driver suddenly to have an arbitrary 
max_sectors, so if no-one objects, I'll pull out the warning.

James

Re: Warning: about not setting max_sectors

[Christoph Hellwig]

On Mon, Dec 09, 2002 at 08:55:41AM -0600, James Bottomley wrote:
> For some reason, the SCSI core is now warning about drivers which don't set 
> max_sectors.
> 
> Just for the record, we have two parameters in the template:
> 
> sg_tablesize which determines the maximum number of scatter gather entries the 
> host can have (corresponds to max_phys_segments in the block layer), and
> 
> max_sectors, which imposes a maximum overall transfer length (corresponds to 
> max_sectors in the block queue).
> 
> The comments over max_sectors list it as optional (if unset, we set it to the 
> scsi equivalent of machine infinity---well, actually 1024, or 512Mb for usual 
> blocks).
> 
> The reason it's unset on so many drivers is that usually, they have no 
> absolute transfer limit, so they're just bounded by the number of entries in 
> the scatter-gather list.
> 
> I can't see a reason to force every driver suddenly to have an arbitrary 
> max_sectors, so if no-one objects, I'll pull out the warning.

I added this intentionally.  The default is set in scsi_register_host
which is an obsolete interface.  This warning reminds driver writers to set
the value explicitly, with scsi_add_host & friends they won't get a default
anymore.

Re: Warning: about not setting max_sectors

[GOTO Masanori]

At Tue, 10 Dec 2002 13:23:48 +0000,
Christoph Hellwig wrote:
> On Mon, Dec 09, 2002 at 08:55:41AM -0600, James Bottomley wrote:
> > For some reason, the SCSI core is now warning about drivers which don't set 
> > max_sectors.
> > 
> > Just for the record, we have two parameters in the template:
> > 
> > sg_tablesize which determines the maximum number of scatter gather entries the 
> > host can have (corresponds to max_phys_segments in the block layer), and
> > 
> > max_sectors, which imposes a maximum overall transfer length (corresponds to 
> > max_sectors in the block queue).
> > 
> > The comments over max_sectors list it as optional (if unset, we set it to the 
> > scsi equivalent of machine infinity---well, actually 1024, or 512Mb for usual 
> > blocks).
> > 
> > The reason it's unset on so many drivers is that usually, they have no 
> > absolute transfer limit, so they're just bounded by the number of entries in 
> > the scatter-gather list.
> > 
> > I can't see a reason to force every driver suddenly to have an arbitrary 
> > max_sectors, so if no-one objects, I'll pull out the warning.
> 
> I added this intentionally.  The default is set in scsi_register_host
> which is an obsolete interface.  This warning reminds driver writers to set
> the value explicitly, with scsi_add_host & friends they won't get a default
> anymore.

Except for explicitly max_sectors defined drivers, the definition of
max_sectors on each drivers' Scsi_Host_Template does not need for a
long time.

So, IMHO, we set .max_sectors (for example) MAX_SECTORS_ALL like
.sg_tablesize = SG_ALL for unaware drivers' Scsi_Host_Template, then
removing such warning. 

-- gotom

Re: Warning: about not setting max_sectors

[James Bottomley]

hch@infradead.org said:
> I added this intentionally.  The default is set in scsi_register_host
> which is an obsolete interface.  This warning reminds driver writers
> to set the value explicitly, with scsi_add_host & friends they won't
> get a default anymore. 

I don't think that's the correct behaviour.  The parameter is labelled as 
optional (and clearly should be for drivers that have no hard transfer limit).

The blk_queue_max_sectors() clearly should be initialised with the device in 
mind.  We have this limit coming at us from two directions: the driver (if it 
imposes such a limit) and the device (e.g. a SCSI-1 device only does read 6 
and thus 256 is the maximum etc.)

we should probably take the lowest of the scsi-1 minimum and the HBA if it 
exists in scsi_initialize_queue and readjust it when we've decided on the 
capabilities of the device.

In any event, moving the zero check for shpnt->max_sectors to 
scsi_initialize_queue would fix the loss of scsi_add_host, wouldn't it?

James

Re: Warning: about not setting max_sectors

[Christoph Hellwig]

On Tue, Dec 10, 2002 at 08:47:50AM -0600, James Bottomley wrote:
> we should probably take the lowest of the scsi-1 minimum and the HBA if it 
> exists in scsi_initialize_queue and readjust it when we've decided on the 
> capabilities of the device.
> 
> In any event, moving the zero check for shpnt->max_sectors to 
> scsi_initialize_queue would fix the loss of scsi_add_host, wouldn't it?

Yepp.  Will you fix it or should I send a patch?

Re: Warning: about not setting max_sectors

[James Bottomley]

hch@infradead.org said:
> Yepp.  Will you fix it or should I send a patch?

I'll do it (I may even look at adjusting the maximum based on the device).

James