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

[Randy.Dunlap]

On Thu, 8 Jun 2006 08:30:00 +0200 Jens Axboe wrote:

> 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?

Obviously not.
(It calls ide_init_drive_cmd(), which clears the <request>.)

Thanks.
---
~Randy
-
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/