Re: [PATCH] ide-cd: use blk_get_request()

[Jens Axboe]

On Wed, Jun 07 2006, Randy.Dunlap wrote:
> On Wed, 7 Jun 2006 08:22:08 +0200 Jens Axboe wrote:
> 
> > On Tue, Jun 06 2006, Andrew Morton wrote:
> > > 
> > > Note that Laurent is also passing through ide_cdrom_packet(), which has a
> > > `struct request' on the stack.  The kernel does this in a lot of places,
> > > and at 168 bytes on x86, it'd really be best if we were to dynamically
> > > allocate these things.
> > 
> > That's an old peeve of mine, on-stack requests... It's nasty from
> > several angles, stack usage just being one of them. Perhaps I'll give it
> > a go for 2.6.18 and add checks for request being thrown at the block
> > layer which didn't originate from get_request().
> 
> This is a start at converting ide-cd.c to use blk_get_request().
> How does it look so far?
> It builds, but I have not tested it yet.
> And of course, there are other drivers to be modified as well.
> 
> ---
> From: Randy Dunlap <rdunlap@xxxxxxxxxxxx>
> 
> Convert struct request req; on function stacks to
> use allocation via blk_get_request() to
> (a) reduce stack pressure and
> (b) use centralized blk_ functions and
> (c) allow for block IO tracing.
> 
> Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxx>
> ---
>  drivers/ide/ide-cd.c |  258 +++++++++++++++++++++++++++++++++------------------
>  1 files changed, 170 insertions(+), 88 deletions(-)
> 
> --- linux-2617-rc6.orig/drivers/ide/ide-cd.c
> +++ linux-2617-rc6/drivers/ide/ide-cd.c
> @@ -2033,24 +2033,32 @@ int msf_to_lba (byte m, byte s, byte f)
>  
>  static int cdrom_check_status(ide_drive_t *drive, struct request_sense *sense)
>  {
> -	struct request req;
> +	struct request *req;
>  	struct cdrom_info *info = drive->driver_data;
>  	struct cdrom_device_info *cdi = &info->devinfo;
> +	request_queue_t *q = cdi->disk->queue;
> +	int stat;
>  
> -	cdrom_prepare_request(drive, &req);
> -
> -	req.sense = sense;
> -	req.cmd[0] = GPCMD_TEST_UNIT_READY;
> -	req.flags |= REQ_QUIET;
> +	req = blk_get_request(q, READ, GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	cdrom_prepare_request(drive, req);

This cannot work, have you seen what cdrom_prepare_request() does?

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/