* [PATCH] Reduce stack usage in sg.c
@ 2005-03-22 8:03 Yum Rayan
2005-03-23 3:56 ` Randy.Dunlap
0 siblings, 1 reply; 3+ messages in thread
From: Yum Rayan @ 2005-03-22 8:03 UTC (permalink / raw)
To: linux-scsi, dgilbert, james.bottomley
Attempt to reduce stack usage in sg.c.
Used checkstack.pl script to measure stack usage and noted follows:
Before patch:
sg_ioctl - 412
sg_read - 200
After patch:
sg_ioctl - 92
sg_read - 96
Thanks,
Rayan
Signed-off-by: Yum Rayan <yum.rayan@gmail.com>
diff -Nur linux-2.6.11.5.orig/drivers/scsi/sg.c linux-2.6.11.5/drivers/scsi/sg.c
--- linux-2.6.11.5.orig/drivers/scsi/sg.c 2005-03-18 22:34:56.000000000 -0800
+++ linux-2.6.11.5/drivers/scsi/sg.c 2005-03-21 23:48:52.000000000 -0800
@@ -335,109 +335,143 @@
Sg_fd *sfp;
Sg_request *srp;
int req_pack_id = -1;
- struct sg_header old_hdr;
- sg_io_hdr_t new_hdr;
+ struct sg_header *old_hdr;
sg_io_hdr_t *hp;
+ int retval = 0;
+
if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
return -ENXIO;
SCSI_LOG_TIMEOUT(3, printk("sg_read: %s, count=%d\n",
sdp->disk->disk_name, (int) count));
- if ((k = verify_area(VERIFY_WRITE, buf, count)))
- return k;
+ old_hdr = kmalloc(SZ_SG_HEADER, GFP_KERNEL);
+ if (!old_hdr)
+ return -ENOMEM;
+ if ((k = verify_area(VERIFY_WRITE, buf, count))) {
+ retval = k;
+ goto free_old_hdr;
+ }
if (sfp->force_packid && (count >= SZ_SG_HEADER)) {
- if (__copy_from_user(&old_hdr, buf, SZ_SG_HEADER))
- return -EFAULT;
- if (old_hdr.reply_len < 0) {
+ if (__copy_from_user(old_hdr, buf, SZ_SG_HEADER)){
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
+ if (old_hdr->reply_len < 0) {
if (count >= SZ_SG_IO_HDR) {
- if (__copy_from_user
- (&new_hdr, buf, SZ_SG_IO_HDR))
- return -EFAULT;
- req_pack_id = new_hdr.pack_id;
+ sg_io_hdr_t *new_hdr;
+ new_hdr = kmalloc(SZ_SG_IO_HDR, GFP_KERNEL);
+ if (!new_hdr) {
+ retval = -ENOMEM;
+ goto free_old_hdr;
+ }
+ retval = __copy_from_user(new_hdr, buf,
+ SZ_SG_IO_HDR);
+ req_pack_id = new_hdr->pack_id;
+ kfree(new_hdr);
+ if (retval) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
}
} else
- req_pack_id = old_hdr.pack_id;
+ req_pack_id = old_hdr->pack_id;
}
srp = sg_get_rq_mark(sfp, req_pack_id);
if (!srp) { /* now wait on packet to arrive */
- if (sdp->detached)
- return -ENODEV;
- if (filp->f_flags & O_NONBLOCK)
- return -EAGAIN;
+ if (sdp->detached) {
+ retval = -ENODEV;
+ goto free_old_hdr;
+ }
+ if (filp->f_flags & O_NONBLOCK) {
+ retval = -EAGAIN;
+ goto free_old_hdr;
+ }
while (1) {
- res = 0; /* following is a macro that beats race condition */
+ res = 0; /* following macro beats race condition */
__wait_event_interruptible(sfp->read_wait,
- (sdp->detached || (srp = sg_get_rq_mark(sfp, req_pack_id))),
- res);
- if (sdp->detached)
- return -ENODEV;
+ (sdp->detached ||
+ (srp = sg_get_rq_mark(sfp, req_pack_id))),
+ res);
+ if (sdp->detached) {
+ retval = -ENODEV;
+ goto free_old_hdr;
+ }
if (0 == res)
break;
- return res; /* -ERESTARTSYS because signal hit process */
+ retval = res; /* -ERESTARTSYS as signal hit process */
+ goto free_old_hdr;
}
}
- if (srp->header.interface_id != '\0')
- return sg_new_read(sfp, buf, count, srp);
+ if (srp->header.interface_id != '\0') {
+ retval = sg_new_read(sfp, buf, count, srp);
+ goto free_old_hdr;
+ }
hp = &srp->header;
- memset(&old_hdr, 0, SZ_SG_HEADER);
- old_hdr.reply_len = (int) hp->timeout;
- old_hdr.pack_len = old_hdr.reply_len; /* very old, strange behaviour */
- old_hdr.pack_id = hp->pack_id;
- old_hdr.twelve_byte =
+ memset(old_hdr, 0, SZ_SG_HEADER);
+ old_hdr->reply_len = (int) hp->timeout;
+ old_hdr->pack_len = old_hdr->reply_len; /* old, strange behaviour */
+ old_hdr->pack_id = hp->pack_id;
+ old_hdr->twelve_byte =
((srp->data.cmd_opcode >= 0xc0) && (12 == hp->cmd_len)) ? 1 : 0;
- old_hdr.target_status = hp->masked_status;
- old_hdr.host_status = hp->host_status;
- old_hdr.driver_status = hp->driver_status;
+ old_hdr->target_status = hp->masked_status;
+ old_hdr->host_status = hp->host_status;
+ old_hdr->driver_status = hp->driver_status;
if ((CHECK_CONDITION & hp->masked_status) ||
(DRIVER_SENSE & hp->driver_status))
- memcpy(old_hdr.sense_buffer, srp->sense_b,
- sizeof (old_hdr.sense_buffer));
+ memcpy(old_hdr->sense_buffer, srp->sense_b,
+ sizeof (old_hdr->sense_buffer));
switch (hp->host_status) {
/* This setup of 'result' is for backward compatibility and is best
ignored by the user who should use target, host + driver status */
case DID_OK:
case DID_PASSTHROUGH:
case DID_SOFT_ERROR:
- old_hdr.result = 0;
+ old_hdr->result = 0;
break;
case DID_NO_CONNECT:
case DID_BUS_BUSY:
case DID_TIME_OUT:
- old_hdr.result = EBUSY;
+ old_hdr->result = EBUSY;
break;
case DID_BAD_TARGET:
case DID_ABORT:
case DID_PARITY:
case DID_RESET:
case DID_BAD_INTR:
- old_hdr.result = EIO;
+ old_hdr->result = EIO;
break;
case DID_ERROR:
- old_hdr.result = (srp->sense_b[0] == 0 &&
+ old_hdr->result = (srp->sense_b[0] == 0 &&
hp->masked_status == GOOD) ? 0 : EIO;
break;
default:
- old_hdr.result = EIO;
+ old_hdr->result = EIO;
break;
}
/* Now copy the result back to the user buffer. */
if (count >= SZ_SG_HEADER) {
- if (__copy_to_user(buf, &old_hdr, SZ_SG_HEADER))
- return -EFAULT;
+ if (__copy_to_user(buf, old_hdr, SZ_SG_HEADER)) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
buf += SZ_SG_HEADER;
- if (count > old_hdr.reply_len)
- count = old_hdr.reply_len;
+ if (count > old_hdr->reply_len)
+ count = old_hdr->reply_len;
if (count > SZ_SG_HEADER) {
if ((res =
sg_read_oxfer(srp, buf, count - SZ_SG_HEADER)))
- return -EFAULT;
+ retval = -EFAULT;
+ goto free_old_hdr;
}
} else
- count = (old_hdr.result == 0) ? 0 : -EIO;
+ count = (old_hdr->result == 0) ? 0 : -EIO;
sg_finish_rem_req(srp);
- return count;
+ retval = count;
+free_old_hdr:
+ kfree(old_hdr);
+ return retval;
}
static ssize_t
@@ -943,8 +977,11 @@
if (result)
return result;
else {
- sg_req_info_t rinfo[SG_MAX_QUEUE];
- Sg_request *srp;
+ sg_req_info_t *rinfo;
+ rinfo = kmalloc(SG_MAX_QUEUE * SZ_SG_REQ_INFO,
+ GFP_KERNEL);
+ if (!rinfo)
+ return -ENOMEM;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
++val, srp = srp ? srp->nextrp : srp) {
@@ -966,8 +1003,11 @@
}
}
read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- return (__copy_to_user(p, rinfo,
- SZ_SG_REQ_INFO * SG_MAX_QUEUE) ? -EFAULT : 0);
+ result = __copy_to_user(p, rinfo, SZ_SG_REQ_INFO *
+ SG_MAX_QUEUE);
+ result = result ? -EFAULT : 0;
+ kfree(rinfo);
+ return result;
}
case SG_EMULATED_HOST:
if (sdp->detached)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Reduce stack usage in sg.c
2005-03-22 8:03 [PATCH] Reduce stack usage in sg.c Yum Rayan
@ 2005-03-23 3:56 ` Randy.Dunlap
2005-03-25 8:20 ` Yum Rayan
0 siblings, 1 reply; 3+ messages in thread
From: Randy.Dunlap @ 2005-03-23 3:56 UTC (permalink / raw)
To: Yum Rayan; +Cc: linux-scsi, dgilbert, james.bottomley
Yum Rayan wrote:
> Attempt to reduce stack usage in sg.c.
>
> Used checkstack.pl script to measure stack usage and noted follows:
>
> Before patch:
> sg_ioctl - 412
> sg_read - 200
>
> After patch:
> sg_ioctl - 92
> sg_read - 96
>
> Thanks,
> Rayan
>
> Signed-off-by: Yum Rayan <yum.rayan@gmail.com>
Mostly looks OK to me. However, I would rather see patches
against the current -bk instead of 2.6.x.y.
This one does not apply to 2.6.12-rc1-bk1.
AFAIK, development is still primarily done against the linus-bk
tree and not the x.y release tree.
Hunk #1 FAILED at 335.
Hunk #2 succeeded at 971 with fuzz 2 (offset -6 lines).
Hunk #3 succeeded at 997 (offset -6 lines).
1 out of 3 hunks FAILED -- saving rejects to file drivers/scsi/sg.c.rej
> diff -Nur linux-2.6.11.5.orig/drivers/scsi/sg.c linux-2.6.11.5/drivers/scsi/sg.c
> --- linux-2.6.11.5.orig/drivers/scsi/sg.c 2005-03-18 22:34:56.000000000 -0800
> +++ linux-2.6.11.5/drivers/scsi/sg.c 2005-03-21 23:48:52.000000000 -0800
> @@ -335,109 +335,143 @@
> Sg_fd *sfp;
> Sg_request *srp;
> int req_pack_id = -1;
> - struct sg_header old_hdr;
> - sg_io_hdr_t new_hdr;
> + struct sg_header *old_hdr;
> sg_io_hdr_t *hp;
> + int retval = 0;
> +
>
> if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
> return -ENXIO;
> SCSI_LOG_TIMEOUT(3, printk("sg_read: %s, count=%d\n",
> sdp->disk->disk_name, (int) count));
> - if ((k = verify_area(VERIFY_WRITE, buf, count)))
> - return k;
> + old_hdr = kmalloc(SZ_SG_HEADER, GFP_KERNEL);
> + if (!old_hdr)
> + return -ENOMEM;
> + if ((k = verify_area(VERIFY_WRITE, buf, count))) {
> + retval = k;
> + goto free_old_hdr;
> + }
> if (sfp->force_packid && (count >= SZ_SG_HEADER)) {
> - if (__copy_from_user(&old_hdr, buf, SZ_SG_HEADER))
> - return -EFAULT;
> - if (old_hdr.reply_len < 0) {
> + if (__copy_from_user(old_hdr, buf, SZ_SG_HEADER)){
> + retval = -EFAULT;
> + goto free_old_hdr;
> + }
> + if (old_hdr->reply_len < 0) {
> if (count >= SZ_SG_IO_HDR) {
> - if (__copy_from_user
> - (&new_hdr, buf, SZ_SG_IO_HDR))
> - return -EFAULT;
> - req_pack_id = new_hdr.pack_id;
> + sg_io_hdr_t *new_hdr;
> + new_hdr = kmalloc(SZ_SG_IO_HDR, GFP_KERNEL);
> + if (!new_hdr) {
> + retval = -ENOMEM;
> + goto free_old_hdr;
> + }
> + retval = __copy_from_user(new_hdr, buf,
> + SZ_SG_IO_HDR);
> + req_pack_id = new_hdr->pack_id;
> + kfree(new_hdr);
> + if (retval) {
> + retval = -EFAULT;
> + goto free_old_hdr;
> + }
> }
> } else
> - req_pack_id = old_hdr.pack_id;
> + req_pack_id = old_hdr->pack_id;
> }
> srp = sg_get_rq_mark(sfp, req_pack_id);
> if (!srp) { /* now wait on packet to arrive */
> - if (sdp->detached)
> - return -ENODEV;
> - if (filp->f_flags & O_NONBLOCK)
> - return -EAGAIN;
> + if (sdp->detached) {
> + retval = -ENODEV;
> + goto free_old_hdr;
> + }
> + if (filp->f_flags & O_NONBLOCK) {
> + retval = -EAGAIN;
> + goto free_old_hdr;
> + }
This patch section looks suspect to me:
Notice that the second '+' sign begins an illegal construct,
probably missing an 'if' line or 2.
Or the '+' should be '-'.
> while (1) {
> - res = 0; /* following is a macro that beats race condition */
> + res = 0; /* following macro beats race condition */
> __wait_event_interruptible(sfp->read_wait,
> - (sdp->detached || (srp = sg_get_rq_mark(sfp, req_pack_id))),
> - res);
> - if (sdp->detached)
> - return -ENODEV;
> + (sdp->detached ||
> + (srp = sg_get_rq_mark(sfp, req_pack_id))),
> + res);
> + if (sdp->detached) {
> + retval = -ENODEV;
> + goto free_old_hdr;
> + }
> if (0 == res)
> break;
> - return res; /* -ERESTARTSYS because signal hit process */
> + retval = res; /* -ERESTARTSYS as signal hit process */
> + goto free_old_hdr;
> }
> }
> - if (srp->header.interface_id != '\0')
> - return sg_new_read(sfp, buf, count, srp);
> + if (srp->header.interface_id != '\0') {
> + retval = sg_new_read(sfp, buf, count, srp);
> + goto free_old_hdr;
> + }
>
> hp = &srp->header;
> - memset(&old_hdr, 0, SZ_SG_HEADER);
> - old_hdr.reply_len = (int) hp->timeout;
> - old_hdr.pack_len = old_hdr.reply_len; /* very old, strange behaviour */
> - old_hdr.pack_id = hp->pack_id;
> - old_hdr.twelve_byte =
> + memset(old_hdr, 0, SZ_SG_HEADER);
> + old_hdr->reply_len = (int) hp->timeout;
> + old_hdr->pack_len = old_hdr->reply_len; /* old, strange behaviour */
> + old_hdr->pack_id = hp->pack_id;
> + old_hdr->twelve_byte =
> ((srp->data.cmd_opcode >= 0xc0) && (12 == hp->cmd_len)) ? 1 : 0;
> - old_hdr.target_status = hp->masked_status;
> - old_hdr.host_status = hp->host_status;
> - old_hdr.driver_status = hp->driver_status;
> + old_hdr->target_status = hp->masked_status;
> + old_hdr->host_status = hp->host_status;
> + old_hdr->driver_status = hp->driver_status;
> if ((CHECK_CONDITION & hp->masked_status) ||
> (DRIVER_SENSE & hp->driver_status))
> - memcpy(old_hdr.sense_buffer, srp->sense_b,
> - sizeof (old_hdr.sense_buffer));
> + memcpy(old_hdr->sense_buffer, srp->sense_b,
> + sizeof (old_hdr->sense_buffer));
> switch (hp->host_status) {
> /* This setup of 'result' is for backward compatibility and is best
> ignored by the user who should use target, host + driver status */
> case DID_OK:
> case DID_PASSTHROUGH:
> case DID_SOFT_ERROR:
> - old_hdr.result = 0;
> + old_hdr->result = 0;
> break;
> case DID_NO_CONNECT:
> case DID_BUS_BUSY:
> case DID_TIME_OUT:
> - old_hdr.result = EBUSY;
> + old_hdr->result = EBUSY;
> break;
> case DID_BAD_TARGET:
> case DID_ABORT:
> case DID_PARITY:
> case DID_RESET:
> case DID_BAD_INTR:
> - old_hdr.result = EIO;
> + old_hdr->result = EIO;
> break;
> case DID_ERROR:
> - old_hdr.result = (srp->sense_b[0] == 0 &&
> + old_hdr->result = (srp->sense_b[0] == 0 &&
> hp->masked_status == GOOD) ? 0 : EIO;
> break;
> default:
> - old_hdr.result = EIO;
> + old_hdr->result = EIO;
> break;
> }
>
> /* Now copy the result back to the user buffer. */
> if (count >= SZ_SG_HEADER) {
> - if (__copy_to_user(buf, &old_hdr, SZ_SG_HEADER))
> - return -EFAULT;
> + if (__copy_to_user(buf, old_hdr, SZ_SG_HEADER)) {
> + retval = -EFAULT;
> + goto free_old_hdr;
> + }
> buf += SZ_SG_HEADER;
> - if (count > old_hdr.reply_len)
> - count = old_hdr.reply_len;
> + if (count > old_hdr->reply_len)
> + count = old_hdr->reply_len;
> if (count > SZ_SG_HEADER) {
> if ((res =
> sg_read_oxfer(srp, buf, count - SZ_SG_HEADER)))
> - return -EFAULT;
> + retval = -EFAULT;
> + goto free_old_hdr;
> }
> } else
> - count = (old_hdr.result == 0) ? 0 : -EIO;
> + count = (old_hdr->result == 0) ? 0 : -EIO;
> sg_finish_rem_req(srp);
> - return count;
> + retval = count;
> +free_old_hdr:
> + kfree(old_hdr);
> + return retval;
> }
>
> static ssize_t
> @@ -943,8 +977,11 @@
> if (result)
> return result;
> else {
> - sg_req_info_t rinfo[SG_MAX_QUEUE];
> - Sg_request *srp;
> + sg_req_info_t *rinfo;
> + rinfo = kmalloc(SG_MAX_QUEUE * SZ_SG_REQ_INFO,
> + GFP_KERNEL);
> + if (!rinfo)
> + return -ENOMEM;
> read_lock_irqsave(&sfp->rq_list_lock, iflags);
> for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
> ++val, srp = srp ? srp->nextrp : srp) {
> @@ -966,8 +1003,11 @@
> }
> }
> read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
> - return (__copy_to_user(p, rinfo,
> - SZ_SG_REQ_INFO * SG_MAX_QUEUE) ? -EFAULT : 0);
> + result = __copy_to_user(p, rinfo, SZ_SG_REQ_INFO *
> + SG_MAX_QUEUE);
> + result = result ? -EFAULT : 0;
> + kfree(rinfo);
> + return result;
> }
> case SG_EMULATED_HOST:
> if (sdp->detached)
> -
--
~Randy
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Reduce stack usage in sg.c
2005-03-23 3:56 ` Randy.Dunlap
@ 2005-03-25 8:20 ` Yum Rayan
0 siblings, 0 replies; 3+ messages in thread
From: Yum Rayan @ 2005-03-25 8:20 UTC (permalink / raw)
To: Randy.Dunlap; +Cc: linux-scsi, dgilbert, james.bottomley
> Mostly looks OK to me. However, I would rather see patches
> against the current -bk instead of 2.6.x.y.
Point noted. Dug around for info on BK. Cloned
http://linux.bkbits.net/linux-2.6 and updated with pull from
bk://linux-scsi.bkbits.net/scsi-misc-2.6. Generated bk patch with -hdu
option. Hope this new little BK grasshoper got it right :)
> This patch section looks suspect to me:
> Notice that the second '+' sign begins an illegal construct,
> probably missing an 'if' line or 2.
> Or the '+' should be '-'.
>
> > while (1) {
> > - res = 0; /* following is a macro that beats race condition */
> > + res = 0; /* following macro beats race condition */
> > __wait_event_interruptible(sfp->read_wait,
> > - (sdp->detached || (srp = sg_get_rq_mark(sfp, req_pack_id))),
> > - res);
> > - if (sdp->detached)
> > - return -ENODEV;
> > + (sdp->detached ||
> > + (srp = sg_get_rq_mark(sfp, req_pack_id))),
> > + res);
> > + if (sdp->detached) {
I adjusted long lines in and around my changes to fit in 80 column
editor. The second "+" is not a statement but the second argument of
the __wait_event_interruptible macro. So this is verified good.
Reworked patch follows.
Regards,
Rayan
Summary: Reduce stack usage in sg_read() and sg_ioctl()
Signed-off-by: Yum Rayan <yum.rayan@gmail.com>
diff -Nru a/drivers/scsi/sg.c b/drivers/scsi/sg.c
--- a/drivers/scsi/sg.c 2005-03-24 00:36:05 -08:00
+++ b/drivers/scsi/sg.c 2005-03-24 00:36:05 -08:00
@@ -330,14 +330,13 @@
static ssize_t
sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
{
- int res;
Sg_device *sdp;
Sg_fd *sfp;
Sg_request *srp;
int req_pack_id = -1;
- struct sg_header old_hdr;
- sg_io_hdr_t new_hdr;
+ struct sg_header *old_hdr;
sg_io_hdr_t *hp;
+ int retval = 0;
if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
return -ENXIO;
@@ -345,99 +344,131 @@
sdp->disk->disk_name, (int) count));
if (!access_ok(VERIFY_WRITE, buf, count))
return -EFAULT;
+ old_hdr = kmalloc(SZ_SG_HEADER, GFP_KERNEL);
+ if (!old_hdr)
+ return -ENOMEM;
if (sfp->force_packid && (count >= SZ_SG_HEADER)) {
- if (__copy_from_user(&old_hdr, buf, SZ_SG_HEADER))
- return -EFAULT;
- if (old_hdr.reply_len < 0) {
+ if (__copy_from_user(old_hdr, buf, SZ_SG_HEADER)) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
+ if (old_hdr->reply_len < 0) {
if (count >= SZ_SG_IO_HDR) {
- if (__copy_from_user
- (&new_hdr, buf, SZ_SG_IO_HDR))
- return -EFAULT;
- req_pack_id = new_hdr.pack_id;
+ sg_io_hdr_t *new_hdr;
+ new_hdr = kmalloc(SZ_SG_IO_HDR, GFP_KERNEL);
+ if (!new_hdr) {
+ retval = -ENOMEM;
+ goto free_old_hdr;
+ }
+ retval =__copy_from_user
+ (new_hdr, buf, SZ_SG_IO_HDR);
+ req_pack_id = new_hdr->pack_id;
+ kfree(new_hdr);
+ if (retval) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
}
} else
- req_pack_id = old_hdr.pack_id;
+ req_pack_id = old_hdr->pack_id;
}
srp = sg_get_rq_mark(sfp, req_pack_id);
if (!srp) { /* now wait on packet to arrive */
- if (sdp->detached)
- return -ENODEV;
- if (filp->f_flags & O_NONBLOCK)
- return -EAGAIN;
+ if (sdp->detached) {
+ retval = -ENODEV;
+ goto free_old_hdr;
+ }
+ if (filp->f_flags & O_NONBLOCK) {
+ retval = -EAGAIN;
+ goto free_old_hdr;
+ }
while (1) {
- res = 0; /* following is a macro that beats race condition */
+ retval = 0; /* following macro beats race condition */
__wait_event_interruptible(sfp->read_wait,
- (sdp->detached || (srp = sg_get_rq_mark(sfp, req_pack_id))),
- res);
- if (sdp->detached)
- return -ENODEV;
- if (0 == res)
+ (sdp->detached ||
+ (srp = sg_get_rq_mark(sfp, req_pack_id))),
+ retval);
+ if (sdp->detached) {
+ retval = -ENODEV;
+ goto free_old_hdr;
+ }
+ if (0 == retval)
break;
- return res; /* -ERESTARTSYS because signal hit process */
+
+ /* -ERESTARTSYS as signal hit process */
+ goto free_old_hdr;
}
}
- if (srp->header.interface_id != '\0')
- return sg_new_read(sfp, buf, count, srp);
+ if (srp->header.interface_id != '\0') {
+ retval = sg_new_read(sfp, buf, count, srp);
+ goto free_old_hdr;
+ }
hp = &srp->header;
- memset(&old_hdr, 0, SZ_SG_HEADER);
- old_hdr.reply_len = (int) hp->timeout;
- old_hdr.pack_len = old_hdr.reply_len; /* very old, strange behaviour */
- old_hdr.pack_id = hp->pack_id;
- old_hdr.twelve_byte =
+ memset(old_hdr, 0, SZ_SG_HEADER);
+ old_hdr->reply_len = (int) hp->timeout;
+ old_hdr->pack_len = old_hdr->reply_len; /* old, strange behaviour */
+ old_hdr->pack_id = hp->pack_id;
+ old_hdr->twelve_byte =
((srp->data.cmd_opcode >= 0xc0) && (12 == hp->cmd_len)) ? 1 : 0;
- old_hdr.target_status = hp->masked_status;
- old_hdr.host_status = hp->host_status;
- old_hdr.driver_status = hp->driver_status;
+ old_hdr->target_status = hp->masked_status;
+ old_hdr->host_status = hp->host_status;
+ old_hdr->driver_status = hp->driver_status;
if ((CHECK_CONDITION & hp->masked_status) ||
(DRIVER_SENSE & hp->driver_status))
- memcpy(old_hdr.sense_buffer, srp->sense_b,
- sizeof (old_hdr.sense_buffer));
+ memcpy(old_hdr->sense_buffer, srp->sense_b,
+ sizeof (old_hdr->sense_buffer));
switch (hp->host_status) {
/* This setup of 'result' is for backward compatibility and is best
ignored by the user who should use target, host + driver status */
case DID_OK:
case DID_PASSTHROUGH:
case DID_SOFT_ERROR:
- old_hdr.result = 0;
+ old_hdr->result = 0;
break;
case DID_NO_CONNECT:
case DID_BUS_BUSY:
case DID_TIME_OUT:
- old_hdr.result = EBUSY;
+ old_hdr->result = EBUSY;
break;
case DID_BAD_TARGET:
case DID_ABORT:
case DID_PARITY:
case DID_RESET:
case DID_BAD_INTR:
- old_hdr.result = EIO;
+ old_hdr->result = EIO;
break;
case DID_ERROR:
- old_hdr.result = (srp->sense_b[0] == 0 &&
+ old_hdr->result = (srp->sense_b[0] == 0 &&
hp->masked_status == GOOD) ? 0 : EIO;
break;
default:
- old_hdr.result = EIO;
+ old_hdr->result = EIO;
break;
}
/* Now copy the result back to the user buffer. */
if (count >= SZ_SG_HEADER) {
- if (__copy_to_user(buf, &old_hdr, SZ_SG_HEADER))
- return -EFAULT;
+ if (__copy_to_user(buf, old_hdr, SZ_SG_HEADER)) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
buf += SZ_SG_HEADER;
- if (count > old_hdr.reply_len)
- count = old_hdr.reply_len;
+ if (count > old_hdr->reply_len)
+ count = old_hdr->reply_len;
if (count > SZ_SG_HEADER) {
- if ((res =
- sg_read_oxfer(srp, buf, count - SZ_SG_HEADER)))
- return -EFAULT;
+ if (sg_read_oxfer(srp, buf, count - SZ_SG_HEADER)) {
+ retval = -EFAULT;
+ goto free_old_hdr;
+ }
}
} else
- count = (old_hdr.result == 0) ? 0 : -EIO;
+ count = (old_hdr->result == 0) ? 0 : -EIO;
sg_finish_rem_req(srp);
- return count;
+ retval = count;
+free_old_hdr:
+ kfree(old_hdr);
+ return retval;
}
static ssize_t
@@ -937,8 +968,11 @@
if (!access_ok(VERIFY_WRITE, p, SZ_SG_REQ_INFO * SG_MAX_QUEUE))
return -EFAULT;
else {
- sg_req_info_t rinfo[SG_MAX_QUEUE];
- Sg_request *srp;
+ sg_req_info_t *rinfo;
+ rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
+ GFP_KERNEL);
+ if (!rinfo)
+ return -ENOMEM;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
++val, srp = srp ? srp->nextrp : srp) {
@@ -954,14 +988,20 @@
jiffies_to_msecs(
jiffies - srp->header.duration);
rinfo[val].orphan = srp->orphan;
- rinfo[val].sg_io_owned = srp->sg_io_owned;
- rinfo[val].pack_id = srp->header.pack_id;
- rinfo[val].usr_ptr = srp->header.usr_ptr;
+ rinfo[val].sg_io_owned =
+ srp->sg_io_owned;
+ rinfo[val].pack_id =
+ srp->header.pack_id;
+ rinfo[val].usr_ptr =
+ srp->header.usr_ptr;
}
}
read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- return (__copy_to_user(p, rinfo,
- SZ_SG_REQ_INFO * SG_MAX_QUEUE) ? -EFAULT : 0);
+ result = __copy_to_user(p, rinfo,
+ SZ_SG_REQ_INFO * SG_MAX_QUEUE);
+ result = result ? -EFAULT : 0;
+ kfree(rinfo);
+ return result;
}
case SG_EMULATED_HOST:
if (sdp->detached)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2005-03-25 8:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-22 8:03 [PATCH] Reduce stack usage in sg.c Yum Rayan
2005-03-23 3:56 ` Randy.Dunlap
2005-03-25 8:20 ` Yum Rayan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox