All of lore.kernel.org
 help / color / mirror / Atom feed
From: Khalid Aziz and Shuah Khan <azizkhan@gonehiking.org>
To: Christoph Hellwig <hch@lst.de>, "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Khalid Aziz <khalid@gonehiking.org>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Nix <nix@esperi.org.uk>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/3] scsi: Provide for avoiding trailing allocation length with VPD inquiries
Date: Tue, 4 Jan 2022 09:39:33 -0700	[thread overview]
Message-ID: <a6a99a33-9124-bbc5-610b-a67d7af76c4a@gonehiking.org> (raw)
In-Reply-To: <20220103082338.GA28606@lst.de>

On 1/3/22 01:23, Christoph Hellwig wrote:
> On Sun, Jan 02, 2022 at 11:23:45PM +0000, Maciej W. Rozycki wrote:
>> Allow SCSI hosts to request avoiding trailing allocation length with VPD
>> inquiries, and use the mechanism to work around an issue with at least
>> some BusLogic MultiMaster host bus adapters and observed with the BT-958
>> model specifically where issuing commands that return less data than
>> provided for causes fatal failures:
> Wouldn't it make more sesnse to hide this quirk insde of
> scsi_vpd_inquiry to also handle the scsi_get_vpd_buf case?  Something
> like:
>
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 211aace69c22c..194a51f772aaa 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -289,8 +289,8 @@ EXPORT_SYMBOL(scsi_track_queue_full);
>    *
>    * Returns size of the vpd page on success or a negative error number.
>    */
> -static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
> -							u8 page, unsigned len)
> +static int __scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
> +		u8 page, unsigned len)
>   {
>   	int result;
>   	unsigned char cmd[16];
> @@ -321,6 +321,20 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
>   	return get_unaligned_be16(&buffer[2]) + 4;
>   }
>   
> +static int scsi_vpd_inquiry(struct scsi_device *sdev, unsigned char *buffer,
> +		u8 page, unsigned len)
> +{
> +	if (sdev->host->no_trailing_allocation_length) {
> +		int ret = __scsi_vpd_inquiry(sdev, buffer, page, min(4U, len));
> +
> +		if (ret < 4)
> +			return ret;
> +		len = min_t(unsigned int, ret, len);
> +	}
> +
> +	return __scsi_vpd_inquiry(sdev, buffer, page, len);
> +}
> +
>   /**
>    * scsi_get_vpd_page - Get Vital Product Data from a SCSI device
>    * @sdev: The device to ask

This is certainly better. It consolidates all the special cases for 
getting VPD pages in one location and ensures no cases are missed.

--
Khalid

  reply	other threads:[~2022-01-04 16:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-02 23:23 [PATCH v3 0/3] Bring the BusLogic host bus adapter driver up to Y2021 Maciej W. Rozycki
2022-01-02 23:23 ` [PATCH v3 1/3] scsi: Provide for avoiding trailing allocation length with VPD inquiries Maciej W. Rozycki
2022-01-03  8:23   ` Christoph Hellwig
2022-01-04 16:39     ` Khalid Aziz and Shuah Khan [this message]
2022-01-04 17:20     ` Maciej W. Rozycki
2022-01-02 23:23 ` [PATCH v3 2/3] scsi: Avoid using reserved length byte " Maciej W. Rozycki
2022-01-02 23:23 ` [PATCH v3 3/3] scsi: Set allocation length to 255 for ATA Information VPD page Maciej W. Rozycki
2022-01-03  4:09   ` Douglas Gilbert
2022-01-03 21:06     ` Maciej W. Rozycki
2022-01-03 21:28       ` Martin K. Petersen
2022-01-04 13:52         ` Maciej W. Rozycki
2022-01-04 17:57           ` Martin K. Petersen
2022-01-06  4:13             ` Martin K. Petersen
2022-01-06  5:21               ` Damien Le Moal
2022-01-07 14:07                 ` Martin K. Petersen
2022-01-07 10:36               ` Maciej W. Rozycki
2022-01-07 14:03                 ` Martin K. Petersen
2022-01-10 12:00                   ` Maciej W. Rozycki
2022-01-10 15:48                     ` Martin K. Petersen
2022-01-03 21:37     ` Martin K. Petersen

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=a6a99a33-9124-bbc5-610b-a67d7af76c4a@gonehiking.org \
    --to=azizkhan@gonehiking.org \
    --cc=hch@lst.de \
    --cc=jejb@linux.ibm.com \
    --cc=khalid@gonehiking.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=macro@orcam.me.uk \
    --cc=martin.petersen@oracle.com \
    --cc=nix@esperi.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.