On Wed, 2019-02-27 at 22:57 -0500, Martin K. Petersen wrote:
> No point in allocating separate memory for the VPD pages we're
> querying. Use the buffer we already allocated.
>
> Signed-off-by: Martin K. Petersen <[email protected]>
> ---
> drivers/scsi/scsi.c | 1 +
> drivers/scsi/sd.c | 48 ++++++++++++++-------------------------------
> 2 files changed, 16 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 7675ff0ca2ea..1d58e466722f 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -341,6 +341,7 @@ static int scsi_vpd_inquiry(struct scsi_device *sdev,
> unsigned char *buffer,
> cmd[3] = len >> 8;
> cmd[4] = len & 0xff;
> cmd[5] = 0; /* Control byte */
> + memset(buffer, 0x0, len);
Why has this memset() call been added? Please explain this change in the
patch description if you want to keep this change.
> /*
> * I'm not convinced we need to try quite this hard to get VPD, but
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index cc175b78b366..df9538e57dcb 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2879,16 +2879,14 @@ static void sd_read_app_tag_own(struct scsi_disk
> *sdkp, unsigned char *buffer)
> * sd_read_block_limits - Query disk device for preferred I/O sizes.
> * @sdkp: disk to query
> */
> -static void sd_read_block_limits(struct scsi_disk *sdkp)
> +static void sd_read_block_limits(struct scsi_disk *sdkp, unsigned char
> *buffer)
> {
> unsigned int sector_sz = sdkp->device->sector_size;
> const int vpd_len = 64;
> - unsigned char *buffer = kmalloc(vpd_len, GFP_KERNEL);
>
> - if (!buffer ||
> - /* Block Limits VPD */
> - scsi_get_vpd_page(sdkp->device, 0xb0, buffer, vpd_len))
> - goto out;
> + /* Block Limits VPD */
> + if (scsi_get_vpd_page(sdkp->device, 0xb0, buffer, vpd_len))
> + return;
>
> blk_queue_io_min(sdkp->disk->queue,
> get_unaligned_be16(&buffer[6]) * sector_sz);
> @@ -2902,7 +2900,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
> sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]);
>
> if (!sdkp->lbpme)
> - goto out;
> + return;
>
> lba_count = get_unaligned_be32(&buffer[20]);
> desc_count = get_unaligned_be32(&buffer[24]);
> @@ -2934,28 +2932,21 @@ static void sd_read_block_limits(struct scsi_disk
> *sdkp)
> sd_config_discard(sdkp, SD_LBP_DISABLE);
> }
> }
> -
> - out:
> - kfree(buffer);
> }
I would appreciate it if the buffer length would be passed as an argument.
That would make it easier to verify sd_read_block_limits() and its callers.
Thanks,
Bart.
