Re: PATCH 2/5: scsi-scan-blist_replun

[Christoph Hellwig <hch@infradead.org>]

On Wed, Apr 21, 2004 at 04:12:34PM +0200, Kurt Garloff wrote:
> 
> Cleanup/Feature
> 
> Remove CONFIG_SCSI_REPORT_LUNS config option.
> Instead provide BLIST_NOREPORTLUN that can be passed as default_dev_flags
> (but also per device if needed).
> Provide BLIST_REPORTLUN2

Can we make this simply BLIST_REPORTLUN?  I always have to think why the
2 is there until I guess this could mean only for scsi <= 2..

> -#ifdef CONFIG_SCSI_REPORT_LUNS
>  /*
>   * max_scsi_report_luns: the maximum number of LUNS that will be
>   * returned from the REPORT LUNS command. 8 times this value must
> @@ -88,13 +87,12 @@ MODULE_PARM_DESC(max_luns,
>   * in practice, the maximum number of LUNs suppored by any device
>   * is about 16k.
>   */
> -static unsigned int max_scsi_report_luns = 128;
> +static unsigned int max_scsi_report_luns = 511;

Hmm, where does this number come from?

>  	/*
> -	 * Only support SCSI-3 and up devices.
> -	 */
> -	if (sdev->scsi_level < SCSI_3)
> +	 * Only support SCSI-3 and up devices if BLIST_NOREPORTLUN is not set.
> +	 * Also allow SCSI-2 if BLIST_REPORTLUN2 is set and host adapter does
> +	 * support more than 8 LUNs.

This comment and the nested if is a little confusing.  Also why shouldn't
the arbitrary SCSI2 limit for BLIST_REPORTLUN2?  I'm sure we won't find
SCSI1 devices supporting it, but there should not be any harm leaving out
that check - and doing so makes the logic much cleaner.

	/*
	 * REPORT_LUN is used by default only for SCSI 3 devices (unless
	 * BLIST_NOREPORTLUN is set).  For other devices BLIST_REPORTLUN
	 * enables it if the HBA does support more than 8 LUNs.

	 if (bflags & BLIST_NOREPORTLUN)
	 	return 1;
	 if (sdev->scsi_level < 3 && sdev->host->max_lun <= 8 &&
	     !(bflags & BLIST_REPORTLUN2))
	     	return 1;

Note that I'd prefer not to add BLIST_NOREPORTLUN if the discussing
about the LUN mapping comes to a useable consensus.