]> www.infradead.org Git - users/hch/misc.git/commitdiff
scsi: sd: Fix build warning in sd_revalidate_disk()
authorAbinash Singh <abinashsinghlalotra@gmail.com>
Mon, 25 Aug 2025 18:39:38 +0000 (00:09 +0530)
committerMartin K. Petersen <martin.petersen@oracle.com>
Sun, 31 Aug 2025 01:18:27 +0000 (21:18 -0400)
A build warning was triggered due to excessive stack usage in
sd_revalidate_disk():

drivers/scsi/sd.c: In function ‘sd_revalidate_disk.isra’:
drivers/scsi/sd.c:3824:1: warning: the frame size of 1160 bytes is larger than 1024 bytes [-Wframe-larger-than=]

This is caused by a large local struct queue_limits (~400B) allocated on
the stack. Replacing it with a heap allocation using kmalloc()
significantly reduces frame usage. Kernel stack is limited (~8 KB), and
allocating large structs on the stack is discouraged.  As the function
already performs heap allocations (e.g. for buffer), this change fits
well.

Fixes: 804e498e0496 ("sd: convert to the atomic queue limits API")
Cc: stable@vger.kernel.org
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Abinash Singh <abinashsinghlalotra@gmail.com>
Link: https://lore.kernel.org/r/20250825183940.13211-2-abinashsinghlalotra@gmail.com
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/sd.c

index 5b8668accf8e8d9af35a0dae9b64115b4639c82d..bf12e23f121213a01df48f1f60cab35e87bdf888 100644 (file)
@@ -3696,10 +3696,10 @@ static int sd_revalidate_disk(struct gendisk *disk)
        struct scsi_disk *sdkp = scsi_disk(disk);
        struct scsi_device *sdp = sdkp->device;
        sector_t old_capacity = sdkp->capacity;
-       struct queue_limits lim;
-       unsigned char *buffer;
+       struct queue_limits *lim = NULL;
+       unsigned char *buffer = NULL;
        unsigned int dev_max;
-       int err;
+       int err = 0;
 
        SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp,
                                      "sd_revalidate_disk\n"));
@@ -3711,6 +3711,10 @@ static int sd_revalidate_disk(struct gendisk *disk)
        if (!scsi_device_online(sdp))
                goto out;
 
+       lim = kmalloc(sizeof(*lim), GFP_KERNEL);
+       if (!lim)
+               goto out;
+
        buffer = kmalloc(SD_BUF_SIZE, GFP_KERNEL);
        if (!buffer) {
                sd_printk(KERN_WARNING, sdkp, "sd_revalidate_disk: Memory "
@@ -3720,14 +3724,14 @@ static int sd_revalidate_disk(struct gendisk *disk)
 
        sd_spinup_disk(sdkp);
 
-       lim = queue_limits_start_update(sdkp->disk->queue);
+       *lim = queue_limits_start_update(sdkp->disk->queue);
 
        /*
         * Without media there is no reason to ask; moreover, some devices
         * react badly if we do.
         */
        if (sdkp->media_present) {
-               sd_read_capacity(sdkp, &lim, buffer);
+               sd_read_capacity(sdkp, lim, buffer);
                /*
                 * Some USB/UAS devices return generic values for mode pages
                 * until the media has been accessed. Trigger a READ operation
@@ -3741,17 +3745,17 @@ static int sd_revalidate_disk(struct gendisk *disk)
                 * cause this to be updated correctly and any device which
                 * doesn't support it should be treated as rotational.
                 */
-               lim.features |= (BLK_FEAT_ROTATIONAL | BLK_FEAT_ADD_RANDOM);
+               lim->features |= (BLK_FEAT_ROTATIONAL | BLK_FEAT_ADD_RANDOM);
 
                if (scsi_device_supports_vpd(sdp)) {
                        sd_read_block_provisioning(sdkp);
-                       sd_read_block_limits(sdkp, &lim);
+                       sd_read_block_limits(sdkp, lim);
                        sd_read_block_limits_ext(sdkp);
-                       sd_read_block_characteristics(sdkp, &lim);
-                       sd_zbc_read_zones(sdkp, &lim, buffer);
+                       sd_read_block_characteristics(sdkp, lim);
+                       sd_zbc_read_zones(sdkp, lim, buffer);
                }
 
-               sd_config_discard(sdkp, &lim, sd_discard_mode(sdkp));
+               sd_config_discard(sdkp, lim, sd_discard_mode(sdkp));
 
                sd_print_capacity(sdkp, old_capacity);
 
@@ -3761,47 +3765,46 @@ 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_config_protection(sdkp, &lim);
+               sd_config_protection(sdkp, lim);
        }
 
        /*
         * We now have all cache related info, determine how we deal
         * with flush requests.
         */
-       sd_set_flush_flag(sdkp, &lim);
+       sd_set_flush_flag(sdkp, lim);
 
        /* Initial block count limit based on CDB TRANSFER LENGTH field size. */
        dev_max = sdp->use_16_for_rw ? SD_MAX_XFER_BLOCKS : SD_DEF_XFER_BLOCKS;
 
        /* Some devices report a maximum block count for READ/WRITE requests. */
        dev_max = min_not_zero(dev_max, sdkp->max_xfer_blocks);
-       lim.max_dev_sectors = logical_to_sectors(sdp, dev_max);
+       lim->max_dev_sectors = logical_to_sectors(sdp, dev_max);
 
        if (sd_validate_min_xfer_size(sdkp))
-               lim.io_min = logical_to_bytes(sdp, sdkp->min_xfer_blocks);
+               lim->io_min = logical_to_bytes(sdp, sdkp->min_xfer_blocks);
        else
-               lim.io_min = 0;
+               lim->io_min = 0;
 
        /*
         * Limit default to SCSI host optimal sector limit if set. There may be
         * an impact on performance for when the size of a request exceeds this
         * host limit.
         */
-       lim.io_opt = sdp->host->opt_sectors << SECTOR_SHIFT;
+       lim->io_opt = sdp->host->opt_sectors << SECTOR_SHIFT;
        if (sd_validate_opt_xfer_size(sdkp, dev_max)) {
-               lim.io_opt = min_not_zero(lim.io_opt,
+               lim->io_opt = min_not_zero(lim->io_opt,
                                logical_to_bytes(sdp, sdkp->opt_xfer_blocks));
        }
 
        sdkp->first_scan = 0;
 
        set_capacity_and_notify(disk, logical_to_sectors(sdp, sdkp->capacity));
-       sd_config_write_same(sdkp, &lim);
-       kfree(buffer);
+       sd_config_write_same(sdkp, lim);
 
-       err = queue_limits_commit_update_frozen(sdkp->disk->queue, &lim);
+       err = queue_limits_commit_update_frozen(sdkp->disk->queue, lim);
        if (err)
-               return err;
+               goto out;
 
        /*
         * Query concurrent positioning ranges after
@@ -3820,7 +3823,10 @@ static int sd_revalidate_disk(struct gendisk *disk)
                set_capacity_and_notify(disk, 0);
 
  out:
-       return 0;
+       kfree(buffer);
+       kfree(lim);
+
+       return err;
 }
 
 /**