From: David Milburn <dmilburn@redhat.com> Date: Wed, 19 Mar 2008 13:13:34 -0500 Subject: [block] sg: cap reserved_size values at max_sectors Message-id: 20080319181334.GA6729@dhcp-210.hsv.redhat.com O-Subject: [RHEL5.2 PATCH] sg: cap reserved_size values at max_sectors Bugzilla: 433481 Steps to Reproduce: # sg_dd if=/dev/sg1 of=/dev/null bpt=128 bs=2048 count=128 Actual results: # sg_dd if=/dev/sg1 of=/dev/null bpt=128 bs=2048 count=128 reading: SCSI status: Check Condition Descriptor format, current; Sense key: Aborted Command Additional sense: No additional sense information Descriptor type: Information 0x0000000000000000 plus...: Driver_status=0x08 [DRIVER_SENSE, SUGGEST_OK] sg_read failed, at or after lba=0 [0x0] Some error occurred, remaining block count=128 0+0 records in 0+0 records out 1 unrecovered read error(s) The problem is that sg_ioctl does not limit the preallocated reserved size to the amount of data that a device may actually transfer in a single transaction. This patch fixes the problem by capping the reserved size to the maximum transfer length. They have verified this patch built against the kernel-2.6.18-81.0.2 sources commit 44ec95425c1d9dce6e4638c29e4362cfb44814e7 Author: Alan Stern <stern@rowland.harvard.edu> Date: Tue Feb 20 11:01:57 2007 -0500 [SCSI] sg: cap reserved_size values at max_sectors This patch (as857) modifies the SG_GET_RESERVED_SIZE and SG_SET_RESERVED_SIZE ioctls in the sg driver, capping the values at the device's request_queue's max_sectors value. This will permit cdrecord to obtain a legal value for the maximum transfer length, fixing Bugzilla #7026. The patch also caps the initial reserved_size value. There's no reason to have a reserved buffer larger than max_sectors, since it would be impossible to use the extra space. The corresponding ioctls in the block layer are modified similarly, and the initial value for the reserved_size is set as large as possible. This will effectively make it default to max_sectors. Note that the actual value is meaningless anyway, since block devices don't have a reserved buffer. Finally, the BLKSECTGET ioctl is added to sg, so that there will be a uniform way for users to determine the actual max_sectors value for any raw SCSI transport. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Acked-by: Jens Axboe <jens.axboe@oracle.com> Acked-by: Douglas Gilbert <dougg@torque.net> Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com> Please review. Thanks, David Acked-by: Doug Ledford <dledford@redhat.com> diff --git a/block/ll_rw_blk.c b/block/ll_rw_blk.c index 3c0034d..26220de 100644 --- a/block/ll_rw_blk.c +++ b/block/ll_rw_blk.c @@ -2007,6 +2007,8 @@ blk_init_queue_node(request_fn_proc *rfn, spinlock_t *lock, int node_id) blk_queue_max_hw_segments(q, MAX_HW_SEGMENTS); blk_queue_max_phys_segments(q, MAX_PHYS_SEGMENTS); + + q->sg_reserved_size = INT_MAX; /* * all done diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c index ed3d3ae..6fea359 100644 --- a/block/scsi_ioctl.c +++ b/block/scsi_ioctl.c @@ -78,7 +78,9 @@ static int sg_set_timeout(request_queue_t *q, int __user *p) static int sg_get_reserved_size(request_queue_t *q, int __user *p) { - return put_user(q->sg_reserved_size, p); + unsigned val = min(q->sg_reserved_size, q->max_sectors << 9); + + return put_user(val, p); } static int sg_set_reserved_size(request_queue_t *q, int __user *p) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 81e3bc7..5bc3b44 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -917,6 +917,8 @@ sg_ioctl(struct inode *inode, struct file *filp, return result; if (val < 0) return -EINVAL; + val = min_t(int, val, + sdp->device->request_queue->max_sectors * 512); if (val != sfp->reserve.bufflen) { if (sg_res_in_use(sfp) || sfp->mmap_called) return -EBUSY; @@ -925,7 +927,8 @@ sg_ioctl(struct inode *inode, struct file *filp, } return 0; case SG_GET_RESERVED_SIZE: - val = (int) sfp->reserve.bufflen; + val = min_t(int, sfp->reserve.bufflen, + sdp->device->request_queue->max_sectors * 512); return put_user(val, ip); case SG_SET_COMMAND_Q: result = get_user(val, ip); @@ -1061,6 +1064,9 @@ sg_ioctl(struct inode *inode, struct file *filp, if (sdp->detached) return -ENODEV; return scsi_ioctl(sdp->device, cmd_in, p); + case BLKSECTGET: + return put_user(sdp->device->request_queue->max_sectors * 512, + ip); default: if (read_only) return -EPERM; /* don't know so take safe approach */ @@ -2339,6 +2345,7 @@ sg_add_sfp(Sg_device * sdp, int dev) { Sg_fd *sfp; unsigned long iflags; + int bufflen; sfp = kzalloc(sizeof(*sfp), GFP_ATOMIC | __GFP_NOWARN); if (!sfp) @@ -2369,7 +2376,9 @@ sg_add_sfp(Sg_device * sdp, int dev) if (unlikely(sg_big_buff != def_reserved_size)) sg_big_buff = def_reserved_size; - sg_build_reserve(sfp, sg_big_buff); + bufflen = min_t(int, sg_big_buff, + sdp->device->request_queue->max_sectors * 512); + sg_build_reserve(sfp, bufflen); SCSI_LOG_TIMEOUT(3, printk("sg_add_sfp: bufflen=%d, k_use_sg=%d\n", sfp->reserve.bufflen, sfp->reserve.k_use_sg)); return sfp;