Status of patch as168 (sysfs readonly max_sectors attribute)

Status of patch as168 (sysfs readonly max_sectors attribute)

[Alan Stern]

I haven't heard any response to my proposed patch to add a readonly sysfs 
attribute file for max_sectors:

http://marc.theaimsgroup.com/?l=linux-scsi&m=107349222601664&w=2

Does that mean you don't want to apply it and it should be done privately 
within usb-storage?  Or has it already been applied without me realizing?

Alan Stern

Re: Status of patch as168 (sysfs readonly max_sectors attribute)

[James Bottomley]

On Thu, 2004-01-15 at 13:03, Alan Stern wrote:
> Does that mean you don't want to apply it and it should be done privately 
> within usb-storage?  Or has it already been applied without me realizing?

Sorry, I meant to comment on this.

The thing I really don't like is the fact that max_sectors is a block
attribute but it's being exported from SCSI, I didn't catch this until I
saw the patch.  This means it's should properly be exported at the block
level, not at the SCSI level.

James

Re: Status of patch as168 (sysfs readonly max_sectors attribute)

[Jeff Garzik]

James Bottomley wrote:
> On Thu, 2004-01-15 at 13:03, Alan Stern wrote:
> 
>>Does that mean you don't want to apply it and it should be done privately 
>>within usb-storage?  Or has it already been applied without me realizing?
> 
> 
> Sorry, I meant to comment on this.
> 
> The thing I really don't like is the fact that max_sectors is a block
> attribute but it's being exported from SCSI, I didn't catch this until I
> saw the patch.  This means it's should properly be exported at the block
> level, not at the SCSI level.


And I'm glad you commented on this :)

Yeah, I want to kill scsi's max_sectors and dma_boundary both.  All SCSI 
does is pass them straight to blk_queue_max_sectors() and 
blk_queue_segment_boundary() from scsi_alloc_queue() anyway.

But it seems to me that the block layer already exports everything needed.

IMO: esteemed and wise SCSI maintainers should pick a point 
(->slave_config?) where the low-level SCSI driver is allowed to tweak a 
queue's settings in a race-free way (i.e. before any requests are 
allowed to be sent to that queue).

Once that is done, elimination of max_sectors and dma_boundary is a 
three-step process:
1) create a helper function scsi_config_queue() or somesuch.  This is 
the default for low-level drivers without a slave_config.
2) make a bombing run through the low-level drivers, removing 
max_sectors and dma_boundary from their template
3) simultaneously to #2, make a similar bombing run, adding or updating 
the slave_config for each driver

Long term I want to bring low-level {scsi | ide | whatever} drivers much 
closer to the block layer, and the above is actually one of the steps I 
wanted to do anyway.

	Jeff

Re: Status of patch as168 (sysfs readonly max_sectors attribute)

[Alan Stern]

On Thu, 15 Jan 2004, Jeff Garzik wrote:

> Yeah, I want to kill scsi's max_sectors and dma_boundary both.  All SCSI 
> does is pass them straight to blk_queue_max_sectors() and 
> blk_queue_segment_boundary() from scsi_alloc_queue() anyway.
> 
> But it seems to me that the block layer already exports everything needed.
> 
> IMO: esteemed and wise SCSI maintainers should pick a point 
> (->slave_config?) where the low-level SCSI driver is allowed to tweak a 
> queue's settings in a race-free way (i.e. before any requests are 
> allowed to be sent to that queue).
> 
> Once that is done, elimination of max_sectors and dma_boundary is a 
> three-step process:
> 1) create a helper function scsi_config_queue() or somesuch.  This is 
> the default for low-level drivers without a slave_config.
> 2) make a bombing run through the low-level drivers, removing 
> max_sectors and dma_boundary from their template
> 3) simultaneously to #2, make a similar bombing run, adding or updating 
> the slave_config for each driver
> 
> Long term I want to bring low-level {scsi | ide | whatever} drivers much 
> closer to the block layer, and the above is actually one of the steps I 
> wanted to do anyway.

If I understand this, you would like to see the low-level drivers manage
these things for themselves by talking directly to the block layer,
without having to go through the intermediate SCSI layers.  This would
imply that any sysfs interface to max_sectors would then be the
responsibility of the driver, since the block layer itself is much too
generic to permit users to fiddle with the settings.

Correct?

Alan Stern

Re: Status of patch as168 (sysfs readonly max_sectors attribute)

[James Bottomley]

On Thu, 2004-01-15 at 14:30, Alan Stern wrote:
> If I understand this, you would like to see the low-level drivers manage
> these things for themselves by talking directly to the block layer,
> without having to go through the intermediate SCSI layers.  This would
> imply that any sysfs interface to max_sectors would then be the
> responsibility of the driver, since the block layer itself is much too
> generic to permit users to fiddle with the settings.
> 
> Correct?

Well, no, not exactly.  At issue are some of the SCSI template
parameters which are copied to the host structure and then simply passed
straight through to the block queue.

If you look in scsi_alloc_queue, it simply passes through these queue
parameters.  It's pretty pointless to have SCSI do anything about them
at all, so Jeff was saying we could just strip this out of the template
and the drivers and have them call the relevant blk_queue_ parameter
functions directly.

James


I personally think this would have to be done *before* any queue
activity, so probably in slave_alloc rather than slave_configure
(although there's no reason not to readjust in slave_configure).

James