diff mbox series

[2/4] scsi: sd: add concurrent positioning ranges support

Message ID 20210721104205.885115-3-damien.lemoal@wdc.com
State New
Headers show
Series Initial support for multi-actuator HDDs | expand

Commit Message

Damien Le Moal July 21, 2021, 10:42 a.m. UTC 
Add the sd_read_cpr() function to the sd scsi disk driver to discover
if a device has multiple concurrent positioning ranges (i.e. multiple
actuators on an HDD). This new function is called from
sd_revalidate_disk() and uses the block layer functions
blk_alloc_cranges() and blk_queue_set_cranges() to set a device
cranges according to the information retrieved from log page B9h,
if the device supports it.

The format of the Concurrent Positioning Ranges VPD page B9h is defined
in section 6.6.6 of SBC-5.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/sd.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/sd.h |  1 +
 2 files changed, 81 insertions(+)

Comments

Dan Carpenter July 21, 2021, 6:01 p.m. UTC | #1 
Hi Damien,

url:    https://github.com/0day-ci/linux/commits/Damien-Le-Moal/Initial-support-for-multi-actuator-HDDs/20210721-185447
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-randconfig-m001-20210720 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/scsi/sd.c:3204 sd_read_cpr() error: uninitialized symbol 'buffer'.

vim +/buffer +3204 drivers/scsi/sd.c

331fc9cf44c011 Damien Le Moal 2021-07-21  3137  static void sd_read_cpr(struct scsi_disk *sdkp)
331fc9cf44c011 Damien Le Moal 2021-07-21  3138  {
331fc9cf44c011 Damien Le Moal 2021-07-21  3139  	unsigned char *buffer, *desc;
                                                                       ^^^^^^
331fc9cf44c011 Damien Le Moal 2021-07-21  3140  	struct blk_cranges *cr = NULL;
331fc9cf44c011 Damien Le Moal 2021-07-21  3141  	unsigned int nr_cpr = 0;
331fc9cf44c011 Damien Le Moal 2021-07-21  3142  	int i, vpd_len, buf_len = SD_BUF_SIZE;
331fc9cf44c011 Damien Le Moal 2021-07-21  3143  
331fc9cf44c011 Damien Le Moal 2021-07-21  3144  	/*
331fc9cf44c011 Damien Le Moal 2021-07-21  3145  	 * We need to have the capacity set first for the block layer to be
331fc9cf44c011 Damien Le Moal 2021-07-21  3146  	 * able to check the ranges.
331fc9cf44c011 Damien Le Moal 2021-07-21  3147  	 */
331fc9cf44c011 Damien Le Moal 2021-07-21  3148  	if (sdkp->first_scan)
331fc9cf44c011 Damien Le Moal 2021-07-21  3149  		return;
331fc9cf44c011 Damien Le Moal 2021-07-21  3150  
331fc9cf44c011 Damien Le Moal 2021-07-21  3151  	if (!sdkp->capacity)
331fc9cf44c011 Damien Le Moal 2021-07-21  3152  		goto out;
                                                                ^^^^^^^^^
This should just return probably?

331fc9cf44c011 Damien Le Moal 2021-07-21  3153  
331fc9cf44c011 Damien Le Moal 2021-07-21  3154  	/*
331fc9cf44c011 Damien Le Moal 2021-07-21  3155  	 * Concurrent Positioning Ranges VPD: there can be at most 256 ranges,
331fc9cf44c011 Damien Le Moal 2021-07-21  3156  	 * leading to a maximum page size of 64 + 256*32 bytes.
331fc9cf44c011 Damien Le Moal 2021-07-21  3157  	 */
331fc9cf44c011 Damien Le Moal 2021-07-21  3158  	buf_len = 64 + 256*32;
331fc9cf44c011 Damien Le Moal 2021-07-21  3159  	buffer = kmalloc(buf_len, GFP_KERNEL);
331fc9cf44c011 Damien Le Moal 2021-07-21  3160  	if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb9, buffer, buf_len))
331fc9cf44c011 Damien Le Moal 2021-07-21  3161  		goto out;
331fc9cf44c011 Damien Le Moal 2021-07-21  3162  
331fc9cf44c011 Damien Le Moal 2021-07-21  3163  	/* We must have at least a 64B header and one 32B range descriptor */
331fc9cf44c011 Damien Le Moal 2021-07-21  3164  	vpd_len = get_unaligned_be16(&buffer[2]) + 3;
331fc9cf44c011 Damien Le Moal 2021-07-21  3165  	if (vpd_len > buf_len || vpd_len < 64 + 32 || (vpd_len & 31)) {
331fc9cf44c011 Damien Le Moal 2021-07-21  3166  		sd_printk(KERN_ERR, sdkp,
331fc9cf44c011 Damien Le Moal 2021-07-21  3167  			  "Invalid Concurrent Positioning Ranges VPD page\n");
331fc9cf44c011 Damien Le Moal 2021-07-21  3168  		goto out;
331fc9cf44c011 Damien Le Moal 2021-07-21  3169  	}
331fc9cf44c011 Damien Le Moal 2021-07-21  3170  
331fc9cf44c011 Damien Le Moal 2021-07-21  3171  	nr_cpr = (vpd_len - 64) / 32;
331fc9cf44c011 Damien Le Moal 2021-07-21  3172  	if (nr_cpr == 1) {
331fc9cf44c011 Damien Le Moal 2021-07-21  3173  		nr_cpr = 0;
331fc9cf44c011 Damien Le Moal 2021-07-21  3174  		goto out;
331fc9cf44c011 Damien Le Moal 2021-07-21  3175  	}
331fc9cf44c011 Damien Le Moal 2021-07-21  3176  
331fc9cf44c011 Damien Le Moal 2021-07-21  3177  	cr = blk_alloc_cranges(sdkp->disk, nr_cpr);
331fc9cf44c011 Damien Le Moal 2021-07-21  3178  	if (!cr) {
331fc9cf44c011 Damien Le Moal 2021-07-21  3179  		nr_cpr = 0;
331fc9cf44c011 Damien Le Moal 2021-07-21  3180  		goto out;
331fc9cf44c011 Damien Le Moal 2021-07-21  3181  	}
331fc9cf44c011 Damien Le Moal 2021-07-21  3182  
331fc9cf44c011 Damien Le Moal 2021-07-21  3183  	desc = &buffer[64];
331fc9cf44c011 Damien Le Moal 2021-07-21  3184  	for (i = 0; i < nr_cpr; i++, desc += 32) {
331fc9cf44c011 Damien Le Moal 2021-07-21  3185  		if (desc[0] != i) {
331fc9cf44c011 Damien Le Moal 2021-07-21  3186  			sd_printk(KERN_ERR, sdkp,
331fc9cf44c011 Damien Le Moal 2021-07-21  3187  				"Invalid Concurrent Positioning Range number\n");
331fc9cf44c011 Damien Le Moal 2021-07-21  3188  			nr_cpr = 0;
331fc9cf44c011 Damien Le Moal 2021-07-21  3189  			break;
331fc9cf44c011 Damien Le Moal 2021-07-21  3190  		}
331fc9cf44c011 Damien Le Moal 2021-07-21  3191  
331fc9cf44c011 Damien Le Moal 2021-07-21  3192  		cr->ranges[i].sector = sd64_to_sectors(sdkp, desc + 8);
331fc9cf44c011 Damien Le Moal 2021-07-21  3193  		cr->ranges[i].nr_sectors = sd64_to_sectors(sdkp, desc + 16);
331fc9cf44c011 Damien Le Moal 2021-07-21  3194  	}
331fc9cf44c011 Damien Le Moal 2021-07-21  3195  
331fc9cf44c011 Damien Le Moal 2021-07-21  3196  out:
331fc9cf44c011 Damien Le Moal 2021-07-21  3197  	blk_queue_set_cranges(sdkp->disk, cr);
331fc9cf44c011 Damien Le Moal 2021-07-21  3198  	if (nr_cpr && sdkp->nr_actuators != nr_cpr) {
331fc9cf44c011 Damien Le Moal 2021-07-21  3199  		sd_printk(KERN_NOTICE, sdkp,
331fc9cf44c011 Damien Le Moal 2021-07-21  3200  			  "%u concurrent positioning ranges\n", nr_cpr);
331fc9cf44c011 Damien Le Moal 2021-07-21  3201  		sdkp->nr_actuators = nr_cpr;
331fc9cf44c011 Damien Le Moal 2021-07-21  3202  	}
331fc9cf44c011 Damien Le Moal 2021-07-21  3203  
331fc9cf44c011 Damien Le Moal 2021-07-21 @3204  	kfree(buffer);
                                                        ^^^^^^^^^^^^^

331fc9cf44c011 Damien Le Moal 2021-07-21  3205  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hannes Reinecke July 22, 2021, 4:14 p.m. UTC | #2 
On 7/21/21 12:42 PM, Damien Le Moal wrote:
> Add the sd_read_cpr() function to the sd scsi disk driver to discover
> if a device has multiple concurrent positioning ranges (i.e. multiple
> actuators on an HDD). This new function is called from
> sd_revalidate_disk() and uses the block layer functions
> blk_alloc_cranges() and blk_queue_set_cranges() to set a device
> cranges according to the information retrieved from log page B9h,
> if the device supports it.
> 
> The format of the Concurrent Positioning Ranges VPD page B9h is defined
> in section 6.6.6 of SBC-5.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>   drivers/scsi/sd.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/scsi/sd.h |  1 +
>   2 files changed, 81 insertions(+)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b8d55af763f9..b1e767a01b9f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3125,6 +3125,85 @@ static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
>   		sdkp->security = 1;
>   }
>   
> +static inline sector_t sd64_to_sectors(struct scsi_disk *sdkp, u8 *buf)
> +{
> +	return logical_to_sectors(sdkp->device, get_unaligned_be64(buf));
> +}
> +
> +/**
> + * sd_read_cpr - Query concurrent positioning ranges
> + * @sdkp:	disk to query
> + */
> +static void sd_read_cpr(struct scsi_disk *sdkp)
> +{
> +	unsigned char *buffer, *desc;
> +	struct blk_cranges *cr = NULL;
> +	unsigned int nr_cpr = 0;
> +	int i, vpd_len, buf_len = SD_BUF_SIZE;
> +
> +	/*
> +	 * We need to have the capacity set first for the block layer to be
> +	 * able to check the ranges.
> +	 */
> +	if (sdkp->first_scan)
> +		return;
> +
> +	if (!sdkp->capacity)
> +		goto out;
> +
> +	/*
> +	 * Concurrent Positioning Ranges VPD: there can be at most 256 ranges,
> +	 * leading to a maximum page size of 64 + 256*32 bytes.
> +	 */
> +	buf_len = 64 + 256*32;
> +	buffer = kmalloc(buf_len, GFP_KERNEL);
> +	if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb9, buffer, buf_len))
> +		goto out;
> +
> +	/* We must have at least a 64B header and one 32B range descriptor */
> +	vpd_len = get_unaligned_be16(&buffer[2]) + 3;
> +	if (vpd_len > buf_len || vpd_len < 64 + 32 || (vpd_len & 31)) {
> +		sd_printk(KERN_ERR, sdkp,
> +			  "Invalid Concurrent Positioning Ranges VPD page\n");
> +		goto out;
> +	}
> +
> +	nr_cpr = (vpd_len - 64) / 32;
> +	if (nr_cpr == 1) {
> +		nr_cpr = 0;
> +		goto out;
> +	}
> +
> +	cr = blk_alloc_cranges(sdkp->disk, nr_cpr);
> +	if (!cr) {
> +		nr_cpr = 0;
> +		goto out;
> +	}
> +
> +	desc = &buffer[64];
> +	for (i = 0; i < nr_cpr; i++, desc += 32) {
> +		if (desc[0] != i) {
> +			sd_printk(KERN_ERR, sdkp,
> +				"Invalid Concurrent Positioning Range number\n");
> +			nr_cpr = 0;
> +			break;
> +		}
> +
> +		cr->ranges[i].sector = sd64_to_sectors(sdkp, desc + 8);
> +		cr->ranges[i].nr_sectors = sd64_to_sectors(sdkp, desc + 16);
> +	}
> +
> +out:
> +	blk_queue_set_cranges(sdkp->disk, cr);

See? We are _are_ creating a new set of ranges.
So why bother updating the old ones?

> +	if (nr_cpr && sdkp->nr_actuators != nr_cpr) {
> +		sd_printk(KERN_NOTICE, sdkp,
> +			  "%u concurrent positioning ranges\n", nr_cpr);
> +		sdkp->nr_actuators = nr_cpr;
> +	}
> +
> +	kfree(buffer);
> +}
> +
>   /*
>    * Determine the device's preferred I/O size for reads and writes
>    * unless the reported value is unreasonably small, large, not a
> @@ -3240,6 +3319,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>   		sd_read_app_tag_own(sdkp, buffer);
>   		sd_read_write_same(sdkp, buffer);
>   		sd_read_security(sdkp, buffer);
> +		sd_read_cpr(sdkp);
>   	}
>   
>   	/*
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index b59136c4125b..2e5932bde43d 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -106,6 +106,7 @@ struct scsi_disk {
>   	u8		protection_type;/* Data Integrity Field */
>   	u8		provisioning_mode;
>   	u8		zeroing_mode;
> +	u8		nr_actuators;		/* Number of actuators */
>   	unsigned	ATO : 1;	/* state of disk ATO bit */
>   	unsigned	cache_override : 1; /* temp override of WCE,RCD */
>   	unsigned	WCE : 1;	/* state of disk WCE bit */
> 
Otherwise:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
Damien Le Moal July 22, 2021, 11:26 p.m. UTC | #3 
On 2021/07/23 1:14, Hannes Reinecke wrote:
> On 7/21/21 12:42 PM, Damien Le Moal wrote:
>> Add the sd_read_cpr() function to the sd scsi disk driver to discover
>> if a device has multiple concurrent positioning ranges (i.e. multiple
>> actuators on an HDD). This new function is called from
>> sd_revalidate_disk() and uses the block layer functions
>> blk_alloc_cranges() and blk_queue_set_cranges() to set a device
>> cranges according to the information retrieved from log page B9h,
>> if the device supports it.
>>
>> The format of the Concurrent Positioning Ranges VPD page B9h is defined
>> in section 6.6.6 of SBC-5.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>> ---
>>   drivers/scsi/sd.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++
>>   drivers/scsi/sd.h |  1 +
>>   2 files changed, 81 insertions(+)
>>
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index b8d55af763f9..b1e767a01b9f 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -3125,6 +3125,85 @@ static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
>>   		sdkp->security = 1;
>>   }
>>   
>> +static inline sector_t sd64_to_sectors(struct scsi_disk *sdkp, u8 *buf)
>> +{
>> +	return logical_to_sectors(sdkp->device, get_unaligned_be64(buf));
>> +}
>> +
>> +/**
>> + * sd_read_cpr - Query concurrent positioning ranges
>> + * @sdkp:	disk to query
>> + */
>> +static void sd_read_cpr(struct scsi_disk *sdkp)
>> +{
>> +	unsigned char *buffer, *desc;
>> +	struct blk_cranges *cr = NULL;
>> +	unsigned int nr_cpr = 0;
>> +	int i, vpd_len, buf_len = SD_BUF_SIZE;
>> +
>> +	/*
>> +	 * We need to have the capacity set first for the block layer to be
>> +	 * able to check the ranges.
>> +	 */
>> +	if (sdkp->first_scan)
>> +		return;
>> +
>> +	if (!sdkp->capacity)
>> +		goto out;
>> +
>> +	/*
>> +	 * Concurrent Positioning Ranges VPD: there can be at most 256 ranges,
>> +	 * leading to a maximum page size of 64 + 256*32 bytes.
>> +	 */
>> +	buf_len = 64 + 256*32;
>> +	buffer = kmalloc(buf_len, GFP_KERNEL);
>> +	if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb9, buffer, buf_len))
>> +		goto out;
>> +
>> +	/* We must have at least a 64B header and one 32B range descriptor */
>> +	vpd_len = get_unaligned_be16(&buffer[2]) + 3;
>> +	if (vpd_len > buf_len || vpd_len < 64 + 32 || (vpd_len & 31)) {
>> +		sd_printk(KERN_ERR, sdkp,
>> +			  "Invalid Concurrent Positioning Ranges VPD page\n");
>> +		goto out;
>> +	}
>> +
>> +	nr_cpr = (vpd_len - 64) / 32;
>> +	if (nr_cpr == 1) {
>> +		nr_cpr = 0;
>> +		goto out;
>> +	}
>> +
>> +	cr = blk_alloc_cranges(sdkp->disk, nr_cpr);
>> +	if (!cr) {
>> +		nr_cpr = 0;
>> +		goto out;
>> +	}
>> +
>> +	desc = &buffer[64];
>> +	for (i = 0; i < nr_cpr; i++, desc += 32) {
>> +		if (desc[0] != i) {
>> +			sd_printk(KERN_ERR, sdkp,
>> +				"Invalid Concurrent Positioning Range number\n");
>> +			nr_cpr = 0;
>> +			break;
>> +		}
>> +
>> +		cr->ranges[i].sector = sd64_to_sectors(sdkp, desc + 8);
>> +		cr->ranges[i].nr_sectors = sd64_to_sectors(sdkp, desc + 16);
>> +	}
>> +
>> +out:
>> +	blk_queue_set_cranges(sdkp->disk, cr);
> 
> See? We are _are_ creating a new set of ranges.
> So why bother updating the old ones?

See my reply to my comments on patch 1. We do not update the old ones. The cases
are:
1) This is the first call, q->cranges is NULL: cr will become q->cranges.
2) q->cranges is already set and cr has the same info as q->cranges (no change):
cr will be freed by blk_queue_set_cranges().
3) q->cranges is already set and cr is different from q-cranges: q->cranges will
be completely dropped and cr become the new q->cranges.

So we never directly update anything inside q->cranges. We just swap in a new
full structure in case of change.

>> +	if (nr_cpr && sdkp->nr_actuators != nr_cpr) {
>> +		sd_printk(KERN_NOTICE, sdkp,
>> +			  "%u concurrent positioning ranges\n", nr_cpr);
>> +		sdkp->nr_actuators = nr_cpr;
>> +	}
>> +
>> +	kfree(buffer);
>> +}
>> +
>>   /*
>>    * Determine the device's preferred I/O size for reads and writes
>>    * unless the reported value is unreasonably small, large, not a
>> @@ -3240,6 +3319,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>>   		sd_read_app_tag_own(sdkp, buffer);
>>   		sd_read_write_same(sdkp, buffer);
>>   		sd_read_security(sdkp, buffer);
>> +		sd_read_cpr(sdkp);
>>   	}
>>   
>>   	/*
>> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
>> index b59136c4125b..2e5932bde43d 100644
>> --- a/drivers/scsi/sd.h
>> +++ b/drivers/scsi/sd.h
>> @@ -106,6 +106,7 @@ struct scsi_disk {
>>   	u8		protection_type;/* Data Integrity Field */
>>   	u8		provisioning_mode;
>>   	u8		zeroing_mode;
>> +	u8		nr_actuators;		/* Number of actuators */
>>   	unsigned	ATO : 1;	/* state of disk ATO bit */
>>   	unsigned	cache_override : 1; /* temp override of WCE,RCD */
>>   	unsigned	WCE : 1;	/* state of disk WCE bit */
>>
> Otherwise:
> 
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> 
> Cheers,
> 
> Hannes
>
diff mbox series

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b8d55af763f9..b1e767a01b9f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3125,6 +3125,85 @@  static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
 		sdkp->security = 1;
 }
 
+static inline sector_t sd64_to_sectors(struct scsi_disk *sdkp, u8 *buf)
+{
+	return logical_to_sectors(sdkp->device, get_unaligned_be64(buf));
+}
+
+/**
+ * sd_read_cpr - Query concurrent positioning ranges
+ * @sdkp:	disk to query
+ */
+static void sd_read_cpr(struct scsi_disk *sdkp)
+{
+	unsigned char *buffer, *desc;
+	struct blk_cranges *cr = NULL;
+	unsigned int nr_cpr = 0;
+	int i, vpd_len, buf_len = SD_BUF_SIZE;
+
+	/*
+	 * We need to have the capacity set first for the block layer to be
+	 * able to check the ranges.
+	 */
+	if (sdkp->first_scan)
+		return;
+
+	if (!sdkp->capacity)
+		goto out;
+
+	/*
+	 * Concurrent Positioning Ranges VPD: there can be at most 256 ranges,
+	 * leading to a maximum page size of 64 + 256*32 bytes.
+	 */
+	buf_len = 64 + 256*32;
+	buffer = kmalloc(buf_len, GFP_KERNEL);
+	if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb9, buffer, buf_len))
+		goto out;
+
+	/* We must have at least a 64B header and one 32B range descriptor */
+	vpd_len = get_unaligned_be16(&buffer[2]) + 3;
+	if (vpd_len > buf_len || vpd_len < 64 + 32 || (vpd_len & 31)) {
+		sd_printk(KERN_ERR, sdkp,
+			  "Invalid Concurrent Positioning Ranges VPD page\n");
+		goto out;
+	}
+
+	nr_cpr = (vpd_len - 64) / 32;
+	if (nr_cpr == 1) {
+		nr_cpr = 0;
+		goto out;
+	}
+
+	cr = blk_alloc_cranges(sdkp->disk, nr_cpr);
+	if (!cr) {
+		nr_cpr = 0;
+		goto out;
+	}
+
+	desc = &buffer[64];
+	for (i = 0; i < nr_cpr; i++, desc += 32) {
+		if (desc[0] != i) {
+			sd_printk(KERN_ERR, sdkp,
+				"Invalid Concurrent Positioning Range number\n");
+			nr_cpr = 0;
+			break;
+		}
+
+		cr->ranges[i].sector = sd64_to_sectors(sdkp, desc + 8);
+		cr->ranges[i].nr_sectors = sd64_to_sectors(sdkp, desc + 16);
+	}
+
+out:
+	blk_queue_set_cranges(sdkp->disk, cr);
+	if (nr_cpr && sdkp->nr_actuators != nr_cpr) {
+		sd_printk(KERN_NOTICE, sdkp,
+			  "%u concurrent positioning ranges\n", nr_cpr);
+		sdkp->nr_actuators = nr_cpr;
+	}
+
+	kfree(buffer);
+}
+
 /*
  * Determine the device's preferred I/O size for reads and writes
  * unless the reported value is unreasonably small, large, not a
@@ -3240,6 +3319,7 @@  static int sd_revalidate_disk(struct gendisk *disk)
 		sd_read_app_tag_own(sdkp, buffer);
 		sd_read_write_same(sdkp, buffer);
 		sd_read_security(sdkp, buffer);
+		sd_read_cpr(sdkp);
 	}
 
 	/*
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index b59136c4125b..2e5932bde43d 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -106,6 +106,7 @@  struct scsi_disk {
 	u8		protection_type;/* Data Integrity Field */
 	u8		provisioning_mode;
 	u8		zeroing_mode;
+	u8		nr_actuators;		/* Number of actuators */
 	unsigned	ATO : 1;	/* state of disk ATO bit */
 	unsigned	cache_override : 1; /* temp override of WCE,RCD */
 	unsigned	WCE : 1;	/* state of disk WCE bit */