SCSI BLIST_*, sdev_bflags, and scsi_device flags

SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Alan Stern]

A fair number of SCSI BLIST bits have duplicate entries in the scsi_device 
structure (in one instance the meaning is inverted):

BLIST_BORKEN			borken
BLIST_SINGLELUN			single_lun
BLIST_USE_10_BYTE_MS		use_10_for_ms
BLIST_MS_SKIP_PAGE_08		skip_ms_page_8
BLIST_MS_SKIP_PAGE_3F		skip_ms_page_3f
BLIST_MS_192_BYTES_FOR_3F	use_192_bytes_for_3f
BLIST_RETRY_HWERROR		retry_hwerror
BLIST_NOT_LOCKABLE		lockable
BLIST_NOSTARTONADD		no_start_on_add

There's no good reason for this duplication.  It's largely historical; at 
the time the device flags were added there was no way to alter the bflags 
value.  Now there is, since sdev_bflags is stored in the scsi_device.

Should we keep this duplication?  Should we remove the device bits and use
the sdev_bflags bitmasks instead?  I presume we don't want to remove the
BLIST entries because they are visible to userspace.  (Although probably
some of them are not used anywhere at all.)

What about things like BLIST_REPORTLUN2 and BLIST_NOREPORTLUN?  They don't
mean anything for scsi_devices, only for scsi_targets.  Nevertheless they
are part of the same BLIST entries.  Again, I don't see any way around
that without affecting userspace.

Alan Stern

Re: SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Patrick Mansfield]

On Thu, Feb 09, 2006 at 02:55:15PM -0500, Alan Stern wrote:
> A fair number of SCSI BLIST bits have duplicate entries in the scsi_device 
> structure (in one instance the meaning is inverted):
> 
> BLIST_BORKEN			borken
> BLIST_SINGLELUN			single_lun
> BLIST_USE_10_BYTE_MS		use_10_for_ms
> BLIST_MS_SKIP_PAGE_08		skip_ms_page_8
> BLIST_MS_SKIP_PAGE_3F		skip_ms_page_3f
> BLIST_MS_192_BYTES_FOR_3F	use_192_bytes_for_3f
> BLIST_RETRY_HWERROR		retry_hwerror
> BLIST_NOT_LOCKABLE		lockable
> BLIST_NOSTARTONADD		no_start_on_add
> 
> There's no good reason for this duplication.  It's largely historical; at 
> the time the device flags were added there was no way to alter the bflags 
> value.  Now there is, since sdev_bflags is stored in the scsi_device.
> 
> Should we keep this duplication?  Should we remove the device bits and use
> the sdev_bflags bitmasks instead?  I presume we don't want to remove the
> BLIST entries because they are visible to userspace.  (Although probably
> some of them are not used anywhere at all.)

I'd like to see the bit fields replaced by sdev_flags masks. Best with a
macro or macros. I don't see why you would get rid of the BLIST_NNN fields
even ignoring user space and devinfo, as you can use them as the mask
values.

Higher level macros would be easier to replace if we ever move the flags
into the starget or such.

I mean if we have:

#define scsi_nostartonadd(sdev)	(sdev->sdev_bflags & BLIST_NOSTARTONADD)

Used as:

	if (scsi_nostartonadd(sdev))
		...

It can easily be changed in the future to:

#define scsi_nostartonadd(sdev)	\
	(sdev->sdev_target->starget_bflags & BLIST_NOSTARTONADD)

> What about things like BLIST_REPORTLUN2 and BLIST_NOREPORTLUN?  They don't
> mean anything for scsi_devices, only for scsi_targets.  Nevertheless they
> are part of the same BLIST entries.  Again, I don't see any way around
> that without affecting userspace.

A lot of the values are really per target, maybe all of them, plus we
have those bit fields that are set by the host driver (or transport).
And the devinfo code treats them as per vendor + product.

IMO store them all in sdev_bflags for now.

For a given target is it allowed per standard, or does hardware exist that
returns a different vendor and product for different LU's?

I don't see anything looking at the SPC 3 for INQUIRY.

It doesn't make much sense to me, but then there are some crazy devices
out there.

-- Patrick Mansfield

Re: SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Alan Stern]

On Thu, 9 Feb 2006, Patrick Mansfield wrote:

> On Thu, Feb 09, 2006 at 02:55:15PM -0500, Alan Stern wrote:
> > A fair number of SCSI BLIST bits have duplicate entries in the scsi_device 
> > structure (in one instance the meaning is inverted):
> > 
> > BLIST_BORKEN			borken
> > BLIST_SINGLELUN			single_lun
> > BLIST_USE_10_BYTE_MS		use_10_for_ms
> > BLIST_MS_SKIP_PAGE_08		skip_ms_page_8
> > BLIST_MS_SKIP_PAGE_3F		skip_ms_page_3f
> > BLIST_MS_192_BYTES_FOR_3F	use_192_bytes_for_3f
> > BLIST_RETRY_HWERROR		retry_hwerror
> > BLIST_NOT_LOCKABLE		lockable
> > BLIST_NOSTARTONADD		no_start_on_add
> > 
> > There's no good reason for this duplication.  It's largely historical; at 
> > the time the device flags were added there was no way to alter the bflags 
> > value.  Now there is, since sdev_bflags is stored in the scsi_device.
> > 
> > Should we keep this duplication?  Should we remove the device bits and use
> > the sdev_bflags bitmasks instead?  I presume we don't want to remove the
> > BLIST entries because they are visible to userspace.  (Although probably
> > some of them are not used anywhere at all.)
> 
> I'd like to see the bit fields replaced by sdev_flags masks. Best with a
> macro or macros. I don't see why you would get rid of the BLIST_NNN fields
> even ignoring user space and devinfo, as you can use them as the mask
> values.
> 
> Higher level macros would be easier to replace if we ever move the flags
> into the starget or such.
> 
> I mean if we have:
> 
> #define scsi_nostartonadd(sdev)	(sdev->sdev_bflags & BLIST_NOSTARTONADD)
> 
> Used as:
> 
> 	if (scsi_nostartonadd(sdev))
> 		...
> 
> It can easily be changed in the future to:
> 
> #define scsi_nostartonadd(sdev)	\
> 	(sdev->sdev_target->starget_bflags & BLIST_NOSTARTONADD)

Makes sense.

> > What about things like BLIST_REPORTLUN2 and BLIST_NOREPORTLUN?  They don't
> > mean anything for scsi_devices, only for scsi_targets.  Nevertheless they
> > are part of the same BLIST entries.  Again, I don't see any way around
> > that without affecting userspace.
> 
> A lot of the values are really per target, maybe all of them, plus we
> have those bit fields that are set by the host driver (or transport).
> And the devinfo code treats them as per vendor + product.

Which BLIST bits exactly are per-target?  Obviously everything with LUN in
the name and probably the two INQUIRY flags.  Anything else?

> IMO store them all in sdev_bflags for now.

In fact, the sdev_bflags and the starget_bflags can be copies of each
other (subject to modification by the host adapter driver, of course).  I
think the accessors should be inline routines, so they can do
type-checking on the argument (scsi_device vs. scsi_target).

Do you mind if I don't bother to write setter routines?

> For a given target is it allowed per standard, or does hardware exist that
> returns a different vendor and product for different LU's?
> 
> I don't see anything looking at the SPC 3 for INQUIRY.
> 
> It doesn't make much sense to me, but then there are some crazy devices
> out there.

I don't know -- I've never heard of such a thing, but that doesn't mean 
much.

Alan Stern

Re: SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Christoph Hellwig]

On Thu, Feb 09, 2006 at 02:55:15PM -0500, Alan Stern wrote:
> A fair number of SCSI BLIST bits have duplicate entries in the scsi_device 
> structure (in one instance the meaning is inverted):
> 
> BLIST_BORKEN			borken
> BLIST_SINGLELUN			single_lun
> BLIST_USE_10_BYTE_MS		use_10_for_ms
> BLIST_MS_SKIP_PAGE_08		skip_ms_page_8
> BLIST_MS_SKIP_PAGE_3F		skip_ms_page_3f
> BLIST_MS_192_BYTES_FOR_3F	use_192_bytes_for_3f
> BLIST_RETRY_HWERROR		retry_hwerror
> BLIST_NOT_LOCKABLE		lockable
> BLIST_NOSTARTONADD		no_start_on_add
> 
> There's no good reason for this duplication.  It's largely historical; at 
> the time the device flags were added there was no way to alter the bflags 
> value.  Now there is, since sdev_bflags is stored in the scsi_device.
> 
> Should we keep this duplication?  Should we remove the device bits and use
> the sdev_bflags bitmasks instead?  I presume we don't want to remove the
> BLIST entries because they are visible to userspace.  (Although probably
> some of them are not used anywhere at all.)
> 
> What about things like BLIST_REPORTLUN2 and BLIST_NOREPORTLUN?  They don't
> mean anything for scsi_devices, only for scsi_targets.  Nevertheless they
> are part of the same BLIST entries.  Again, I don't see any way around
> that without affecting userspace.

Do we need per-device blist flags at all?  I suspect just having them in
the target should be enough.

Re: SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Alan Stern]

On Fri, 10 Feb 2006, Christoph Hellwig wrote:

> > What about things like BLIST_REPORTLUN2 and BLIST_NOREPORTLUN?  They don't
> > mean anything for scsi_devices, only for scsi_targets.  Nevertheless they
> > are part of the same BLIST entries.  Again, I don't see any way around
> > that without affecting userspace.
> 
> Do we need per-device blist flags at all?  I suspect just having them in
> the target should be enough.

Some of the blist flags clearly are per-device: BLIST_KEY, BLIST_ISROM,
BLIST_NOSTARTONADD, BLIST_MS_SKIP_PAGE_08, BLIST_MS_SKIP_PAGE_3F,
BLIST_USE_10_BYTE_MS, BLIST_MS_192_BYTES_FOR_3F, BLIST_NOT_LOCKABLE,
BLIST_NO_ULD_ATTACH, BLIST_RETRY_HWERROR.  There are a few others I'm not
certain about.

Alan Stern

Re: SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Matthew Wilcox]

On Fri, Feb 10, 2006 at 10:27:21AM -0500, Alan Stern wrote:
> On Fri, 10 Feb 2006, Christoph Hellwig wrote:
> > Do we need per-device blist flags at all?  I suspect just having them in
> > the target should be enough.
> 
> Some of the blist flags clearly are per-device: BLIST_KEY, BLIST_ISROM,
> BLIST_NOSTARTONADD, BLIST_MS_SKIP_PAGE_08, BLIST_MS_SKIP_PAGE_3F,
> BLIST_USE_10_BYTE_MS, BLIST_MS_192_BYTES_FOR_3F, BLIST_NOT_LOCKABLE,
> BLIST_NO_ULD_ATTACH, BLIST_RETRY_HWERROR.  There are a few others I'm not
> certain about.

Why are these clearly per LUN?  I agree some of them are debatable,
but most of them indicate a general lack-of-scsi-spec compliance on the
part of the manufacturer and hence apply to the piece of hardware (==
scsi target) rather than just one of the LUNs in it.

Re: SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Alan Stern]

On Fri, 10 Feb 2006, Matthew Wilcox wrote:

> On Fri, Feb 10, 2006 at 10:27:21AM -0500, Alan Stern wrote:
> > On Fri, 10 Feb 2006, Christoph Hellwig wrote:
> > > Do we need per-device blist flags at all?  I suspect just having them in
> > > the target should be enough.
> > 
> > Some of the blist flags clearly are per-device: BLIST_KEY, BLIST_ISROM,
> > BLIST_NOSTARTONADD, BLIST_MS_SKIP_PAGE_08, BLIST_MS_SKIP_PAGE_3F,
> > BLIST_USE_10_BYTE_MS, BLIST_MS_192_BYTES_FOR_3F, BLIST_NOT_LOCKABLE,
> > BLIST_NO_ULD_ATTACH, BLIST_RETRY_HWERROR.  There are a few others I'm not
> > certain about.
> 
> Why are these clearly per LUN?  I agree some of them are debatable,
> but most of them indicate a general lack-of-scsi-spec compliance on the
> part of the manufacturer and hence apply to the piece of hardware (==
> scsi target) rather than just one of the LUNs in it.

Isn't is possible for a target to contain LUNs made by different 
manufacturers, each with its own kind of lack-of-compliance?

Or to put it another way...  Somewhere the SCSI commands have to be 
decoded and carried out.  With some devices that may be done at the target 
level, and with others it may be done at the LUN level.  So any BLIST flag 
that affects which commands get used should be per-LUN, for greatest 
generality.

For that matter, consider BLIST_ISROM (which means that the device is a 
cdrom).  Obviously that applies to a LUN and not to a target; there's no 
requirement that all the logical units in a target be the same kind of 
device.

Alan Stern

Re: SCSI BLIST_*, sdev_bflags, and scsi_device flags

[Patrick Mansfield]

On Thu, Feb 09, 2006 at 10:00:52PM -0500, Alan Stern wrote:
> On Thu, 9 Feb 2006, Patrick Mansfield wrote:

> Which BLIST bits exactly are per-target?  Obviously everything with LUN in
> the name and probably the two INQUIRY flags.  Anything else?

Per other posts and whether the vendor and product can change across LU's
on a given target, they could all be per-target, but we can't assume
that. I didn't audit the BLIST values.

> > IMO store them all in sdev_bflags for now.
> 
> In fact, the sdev_bflags and the starget_bflags can be copies of each
> other (subject to modification by the host adapter driver, of course).  I
> think the accessors should be inline routines, so they can do
> type-checking on the argument (scsi_device vs. scsi_target).

I was thinking that too, but then you have to get values out of the sdev
flags, and don't need starget flags except to pass bflags around during
scan.

So, just use a per sdev flag.

> Do you mind if I don't bother to write setter routines?

I don't care. hch or James?

I think they would only be used by host drivers or transports, as the scan
would set sdev_bflags = scsi_get_device_flags().

-- Patrick Mansfield