[PATCH v2 00/13] removing the on-stack struct request

[PATCH v2 00/13] removing the on-stack struct request

[FUJITA Tomonori]

This is an updated version of the patchset to clean up the asymmetry
of blk_get/put_request usage:

http://marc.info/?l=linux-ide&m=120882410712466&w=2

This patchset removes the code calling blk_put_request against the
requests that are not allocated via blk_get_request. They can use
__GFP_WAIT allocation so we can easily convert them to
use blk_get/put_request properly.

This patchset enables us to remove the following hack in
blk_put_request:

/*
 * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
 * following if (q) test.
 */
if (q) {
	spin_lock_irqsave(q->queue_lock, flags);
	__blk_put_request(q, req);
	spin_unlock_irqrestore(q->queue_lock, flags);
}

The major changes are:

- I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
the same thing that blk_execute_rq() does. So let's simply use
blk_execute_rq(). The nice side effect is that I unexport
blk_end_sync_rq() since I converted all the users.

- I drop the unnecessary patch to make ide_do_drive_cmd return
rq->errors:

http://marc.info/?l=linux-ide&m=120882410612456&w=2

#1-10 are for the ide subsystem and #11-13 for the block layer. This
is against the lastest Linus tree.

[PATCH v2 01/13] ide-cd: convert ide_cd_queue_pc to use blk_execute_rq

[FUJITA Tomonori]

This converts ide_cd_queue_pc to use blk_execute_rq, necessitating
changing the ide_cd_queue_pc prototype into a form that doesn't takes
a pointer to request struct. ide_cd_queue_pc works like scsi_execute.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd.c       |   90 +++++++++++++++++++++-------------------
 drivers/ide/ide-cd.h       |    3 +-
 drivers/ide/ide-cd_ioctl.c |   99 ++++++++++++++++++++-----------------------
 3 files changed, 95 insertions(+), 97 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 68e7f19..cb573b9 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -838,34 +838,49 @@ static void ide_cd_request_sense_fixup(struct request *rq)
 		}
 }
 
-int ide_cd_queue_pc(ide_drive_t *drive, struct request *rq)
+int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char *cmd,
+		    int write, void *buffer, unsigned bufflen,
+		    struct request_sense *sense, int timeout,
+		    unsigned int cmd_flags)
 {
-	struct request_sense sense;
+	struct cdrom_info *info = drive->driver_data;
+	struct request_sense local_sense;
 	int retries = 10;
-	unsigned int flags = rq->cmd_flags;
+	unsigned int flags = 0;
 
-	if (rq->sense == NULL)
-		rq->sense = &sense;
+	if (!sense)
+		sense = &local_sense;
 
 	/* start of retry loop */
 	do {
+		struct request *rq;
 		int error;
-		unsigned long time = jiffies;
-		rq->cmd_flags = flags;
 
-		error = ide_do_drive_cmd(drive, rq, ide_wait);
-		time = jiffies - time;
+		rq = blk_get_request(drive->queue, write, __GFP_WAIT);
+
+		memcpy(rq->cmd, cmd, BLK_MAX_CDB);
+		rq->cmd_type = REQ_TYPE_ATA_PC;
+		rq->sense = sense;
+		rq->cmd_flags |= cmd_flags;
+		rq->data = buffer;
+		rq->data_len = bufflen;
+		rq->timeout = timeout;
+
+		error = blk_execute_rq(drive->queue, info->disk, rq, 0);
+
+		flags = rq->cmd_flags;
+		blk_put_request(rq);
 
 		/*
 		 * FIXME: we should probably abort/retry or something in case of
 		 * failure.
 		 */
-		if (rq->cmd_flags & REQ_FAILED) {
+		if (flags & REQ_FAILED) {
 			/*
 			 * The request failed.  Retry if it was due to a unit
 			 * attention status (usually means media was changed).
 			 */
-			struct request_sense *reqbuf = rq->sense;
+			struct request_sense *reqbuf = sense;
 
 			if (reqbuf->sense_key == UNIT_ATTENTION)
 				cdrom_saw_media_change(drive);
@@ -885,10 +900,10 @@ int ide_cd_queue_pc(ide_drive_t *drive, struct request *rq)
 		}
 
 		/* end of retry loop */
-	} while ((rq->cmd_flags & REQ_FAILED) && retries >= 0);
+	} while ((flags & REQ_FAILED) && retries >= 0);
 
 	/* return an error if the command failed */
-	return (rq->cmd_flags & REQ_FAILED) ? -EIO : 0;
+	return (flags & REQ_FAILED) ? -EIO : 0;
 }
 
 /*
@@ -1268,23 +1283,20 @@ static void msf_from_bcd(struct atapi_msf *msf)
 
 int cdrom_check_status(ide_drive_t *drive, struct request_sense *sense)
 {
-	struct request req;
 	struct cdrom_info *info = drive->driver_data;
 	struct cdrom_device_info *cdi = &info->devinfo;
+	unsigned char cmd[BLK_MAX_CDB];
 
-	ide_cd_init_rq(drive, &req);
-
-	req.sense = sense;
-	req.cmd[0] = GPCMD_TEST_UNIT_READY;
-	req.cmd_flags |= REQ_QUIET;
+	memset(cmd, 0, BLK_MAX_CDB);
+	cmd[0] = GPCMD_TEST_UNIT_READY;
 
 	/*
 	 * Sanyo 3 CD changer uses byte 7 of TEST_UNIT_READY to switch CDs
 	 * instead of supporting the LOAD_UNLOAD opcode.
 	 */
-	req.cmd[7] = cdi->sanyo_slot % 3;
+	cmd[7] = cdi->sanyo_slot % 3;
 
-	return ide_cd_queue_pc(drive, &req);
+	return ide_cd_queue_pc(drive, cmd, 0, NULL, 0, sense, 0, REQ_QUIET);
 }
 
 static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
@@ -1297,17 +1309,13 @@ static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
 	} capbuf;
 
 	int stat;
-	struct request req;
-
-	ide_cd_init_rq(drive, &req);
+	unsigned char cmd[BLK_MAX_CDB];
 
-	req.sense = sense;
-	req.cmd[0] = GPCMD_READ_CDVD_CAPACITY;
-	req.data = (char *)&capbuf;
-	req.data_len = sizeof(capbuf);
-	req.cmd_flags |= REQ_QUIET;
+	memset(cmd, 0, BLK_MAX_CDB);
+	cmd[0] = GPCMD_READ_CDVD_CAPACITY;
 
-	stat = ide_cd_queue_pc(drive, &req);
+	stat = ide_cd_queue_pc(drive, cmd, 0, &capbuf, sizeof(capbuf),
+			       sense, 0, REQ_QUIET);
 	if (stat == 0) {
 		*capacity = 1 + be32_to_cpu(capbuf.lba);
 		*sectors_per_frame =
@@ -1321,24 +1329,20 @@ static int cdrom_read_tocentry(ide_drive_t *drive, int trackno, int msf_flag,
 				int format, char *buf, int buflen,
 				struct request_sense *sense)
 {
-	struct request req;
+	unsigned char cmd[BLK_MAX_CDB];
 
-	ide_cd_init_rq(drive, &req);
+	memset(cmd, 0, BLK_MAX_CDB);
 
-	req.sense = sense;
-	req.data =  buf;
-	req.data_len = buflen;
-	req.cmd_flags |= REQ_QUIET;
-	req.cmd[0] = GPCMD_READ_TOC_PMA_ATIP;
-	req.cmd[6] = trackno;
-	req.cmd[7] = (buflen >> 8);
-	req.cmd[8] = (buflen & 0xff);
-	req.cmd[9] = (format << 6);
+	cmd[0] = GPCMD_READ_TOC_PMA_ATIP;
+	cmd[6] = trackno;
+	cmd[7] = (buflen >> 8);
+	cmd[8] = (buflen & 0xff);
+	cmd[9] = (format << 6);
 
 	if (msf_flag)
-		req.cmd[1] = 2;
+		cmd[1] = 2;
 
-	return ide_cd_queue_pc(drive, &req);
+	return ide_cd_queue_pc(drive, cmd, 0, buf, buflen, sense, 0, REQ_QUIET);
 }
 
 /* Try to read the entire TOC for the disk into our internal buffer. */
diff --git a/drivers/ide/ide-cd.h b/drivers/ide/ide-cd.h
index a58801c..13918c5 100644
--- a/drivers/ide/ide-cd.h
+++ b/drivers/ide/ide-cd.h
@@ -144,7 +144,8 @@ void ide_cd_log_error(const char *, struct request *, struct request_sense *);
 
 /* ide-cd.c functions used by ide-cd_ioctl.c */
 void ide_cd_init_rq(ide_drive_t *, struct request *);
-int ide_cd_queue_pc(ide_drive_t *, struct request *);
+int ide_cd_queue_pc(ide_drive_t *, const unsigned char *, int, void *, unsigned,
+		    struct request_sense *, int, unsigned int);
 int ide_cd_read_toc(ide_drive_t *, struct request_sense *);
 int ide_cdrom_get_capabilities(ide_drive_t *, u8 *);
 void ide_cdrom_update_speed(ide_drive_t *, u8 *);
diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
index 6d147ce..9c044da 100644
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -104,8 +104,8 @@ int cdrom_eject(ide_drive_t *drive, int ejectflag,
 {
 	struct cdrom_info *cd = drive->driver_data;
 	struct cdrom_device_info *cdi = &cd->devinfo;
-	struct request req;
 	char loej = 0x02;
+	unsigned char cmd[BLK_MAX_CDB];
 
 	if ((cd->cd_flags & IDE_CD_FLAG_NO_EJECT) && !ejectflag)
 		return -EDRIVE_CANT_DO_THIS;
@@ -114,17 +114,16 @@ int cdrom_eject(ide_drive_t *drive, int ejectflag,
 	if ((cd->cd_flags & IDE_CD_FLAG_DOOR_LOCKED) && ejectflag)
 		return 0;
 
-	ide_cd_init_rq(drive, &req);
-
 	/* only tell drive to close tray if open, if it can do that */
 	if (ejectflag && (cdi->mask & CDC_CLOSE_TRAY))
 		loej = 0;
 
-	req.sense = sense;
-	req.cmd[0] = GPCMD_START_STOP_UNIT;
-	req.cmd[4] = loej | (ejectflag != 0);
+	memset(cmd, 0, BLK_MAX_CDB);
+
+	cmd[0] = GPCMD_START_STOP_UNIT;
+	cmd[4] = loej | (ejectflag != 0);
 
-	return ide_cd_queue_pc(drive, &req);
+	return ide_cd_queue_pc(drive, cmd, 0, NULL, 0, sense, 0, 0);
 }
 
 /* Lock the door if LOCKFLAG is nonzero; unlock it otherwise. */
@@ -134,7 +133,6 @@ int ide_cd_lockdoor(ide_drive_t *drive, int lockflag,
 {
 	struct cdrom_info *cd = drive->driver_data;
 	struct request_sense my_sense;
-	struct request req;
 	int stat;
 
 	if (sense == NULL)
@@ -144,11 +142,15 @@ int ide_cd_lockdoor(ide_drive_t *drive, int lockflag,
 	if (cd->cd_flags & IDE_CD_FLAG_NO_DOORLOCK) {
 		stat = 0;
 	} else {
-		ide_cd_init_rq(drive, &req);
-		req.sense = sense;
-		req.cmd[0] = GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL;
-		req.cmd[4] = lockflag ? 1 : 0;
-		stat = ide_cd_queue_pc(drive, &req);
+		unsigned char cmd[BLK_MAX_CDB];
+
+		memset(cmd, 0, BLK_MAX_CDB);
+
+		cmd[0] = GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL;
+		cmd[4] = lockflag ? 1 : 0;
+
+		stat = ide_cd_queue_pc(drive, cmd, 0, NULL, 0,
+				       sense, 0, 0);
 	}
 
 	/* If we got an illegal field error, the drive
@@ -206,32 +208,30 @@ int ide_cdrom_select_speed(struct cdrom_device_info *cdi, int speed)
 {
 	ide_drive_t *drive = cdi->handle;
 	struct cdrom_info *cd = drive->driver_data;
-	struct request rq;
 	struct request_sense sense;
 	u8 buf[ATAPI_CAPABILITIES_PAGE_SIZE];
 	int stat;
-
-	ide_cd_init_rq(drive, &rq);
-
-	rq.sense = &sense;
+	unsigned char cmd[BLK_MAX_CDB];
 
 	if (speed == 0)
 		speed = 0xffff; /* set to max */
 	else
 		speed *= 177;   /* Nx to kbytes/s */
 
-	rq.cmd[0] = GPCMD_SET_SPEED;
+	memset(cmd, 0, BLK_MAX_CDB);
+
+	cmd[0] = GPCMD_SET_SPEED;
 	/* Read Drive speed in kbytes/second MSB/LSB */
-	rq.cmd[2] = (speed >> 8) & 0xff;
-	rq.cmd[3] = speed & 0xff;
+	cmd[2] = (speed >> 8) & 0xff;
+	cmd[3] = speed & 0xff;
 	if ((cdi->mask & (CDC_CD_R | CDC_CD_RW | CDC_DVD_R)) !=
 	    (CDC_CD_R | CDC_CD_RW | CDC_DVD_R)) {
 		/* Write Drive speed in kbytes/second MSB/LSB */
-		rq.cmd[4] = (speed >> 8) & 0xff;
-		rq.cmd[5] = speed & 0xff;
+		cmd[4] = (speed >> 8) & 0xff;
+		cmd[5] = speed & 0xff;
 	}
 
-	stat = ide_cd_queue_pc(drive, &rq);
+	stat = ide_cd_queue_pc(drive, cmd, 0, NULL, 0, &sense, 0, 0);
 
 	if (!ide_cdrom_get_capabilities(drive, buf)) {
 		ide_cdrom_update_speed(drive, buf);
@@ -268,21 +268,18 @@ int ide_cdrom_get_mcn(struct cdrom_device_info *cdi,
 {
 	ide_drive_t *drive = cdi->handle;
 	int stat, mcnlen;
-	struct request rq;
 	char buf[24];
+	unsigned char cmd[BLK_MAX_CDB];
 
-	ide_cd_init_rq(drive, &rq);
+	memset(cmd, 0, BLK_MAX_CDB);
 
-	rq.data = buf;
-	rq.data_len = sizeof(buf);
+	cmd[0] = GPCMD_READ_SUBCHANNEL;
+	cmd[1] = 2;		/* MSF addressing */
+	cmd[2] = 0x40;	/* request subQ data */
+	cmd[3] = 2;		/* format */
+	cmd[8] = sizeof(buf);
 
-	rq.cmd[0] = GPCMD_READ_SUBCHANNEL;
-	rq.cmd[1] = 2;		/* MSF addressing */
-	rq.cmd[2] = 0x40;	/* request subQ data */
-	rq.cmd[3] = 2;		/* format */
-	rq.cmd[8] = sizeof(buf);
-
-	stat = ide_cd_queue_pc(drive, &rq);
+	stat = ide_cd_queue_pc(drive, cmd, 0, buf, sizeof(buf), NULL, 0, 0);
 	if (stat)
 		return stat;
 
@@ -351,8 +348,8 @@ static int ide_cd_fake_play_trkind(ide_drive_t *drive, void *arg)
 	struct atapi_toc_entry *first_toc, *last_toc;
 	unsigned long lba_start, lba_end;
 	int stat;
-	struct request rq;
 	struct request_sense sense;
+	unsigned char cmd[BLK_MAX_CDB];
 
 	stat = ide_cd_get_toc_entry(drive, ti->cdti_trk0, &first_toc);
 	if (stat)
@@ -370,14 +367,13 @@ static int ide_cd_fake_play_trkind(ide_drive_t *drive, void *arg)
 	if (lba_end <= lba_start)
 		return -EINVAL;
 
-	ide_cd_init_rq(drive, &rq);
+	memset(cmd, 0, BLK_MAX_CDB);
 
-	rq.sense = &sense;
-	rq.cmd[0] = GPCMD_PLAY_AUDIO_MSF;
-	lba_to_msf(lba_start,   &rq.cmd[3], &rq.cmd[4], &rq.cmd[5]);
-	lba_to_msf(lba_end - 1, &rq.cmd[6], &rq.cmd[7], &rq.cmd[8]);
+	cmd[0] = GPCMD_PLAY_AUDIO_MSF;
+	lba_to_msf(lba_start,   &cmd[3], &cmd[4], &cmd[5]);
+	lba_to_msf(lba_end - 1, &cmd[6], &cmd[7], &cmd[8]);
 
-	return ide_cd_queue_pc(drive, &rq);
+	return ide_cd_queue_pc(drive, cmd, 0, NULL, 0, &sense, 0, 0);
 }
 
 static int ide_cd_read_tochdr(ide_drive_t *drive, void *arg)
@@ -447,8 +443,8 @@ int ide_cdrom_audio_ioctl(struct cdrom_device_info *cdi,
 int ide_cdrom_packet(struct cdrom_device_info *cdi,
 			    struct packet_command *cgc)
 {
-	struct request req;
 	ide_drive_t *drive = cdi->handle;
+	unsigned int flags = 0;
 
 	if (cgc->timeout <= 0)
 		cgc->timeout = ATAPI_WAIT_PC;
@@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
 	/* here we queue the commands from the uniform CD-ROM
 	   layer. the packet must be complete, as we do not
 	   touch it at all. */
-	ide_cd_init_rq(drive, &req);
 
 	if (cgc->data_direction == CGC_DATA_WRITE)
-		req.cmd_flags |= REQ_RW;
+		flags |= REQ_RW;
 
-	memcpy(req.cmd, cgc->cmd, CDROM_PACKET_SIZE);
 	if (cgc->sense)
 		memset(cgc->sense, 0, sizeof(struct request_sense));
-	req.data = cgc->buffer;
-	req.data_len = cgc->buflen;
-	req.timeout = cgc->timeout;
 
 	if (cgc->quiet)
-		req.cmd_flags |= REQ_QUIET;
+		flags |= REQ_QUIET;
 
-	req.sense = cgc->sense;
-	cgc->stat = ide_cd_queue_pc(drive, &req);
+	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
+				    cgc->data_direction == CGC_DATA_WRITE,
+				    cgc->buffer, cgc->buflen,
+				    cgc->sense, cgc->timeout, flags);
 	if (!cgc->stat)
-		cgc->buflen -= req.data_len;
+		cgc->buflen = 0;
 	return cgc->stat;
 }
-- 
1.5.4.2

[PATCH v2 02/13] ide-cd: convert ide_do_drive_cmd path to use blk_execute_rq

[FUJITA Tomonori]

This converts the ide_do_drive_cmd path using ide_wait to use
blk_execute_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd_ioctl.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
index 9c044da..4571b4f 100644
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -295,14 +295,14 @@ int ide_cdrom_reset(struct cdrom_device_info *cdi)
 	ide_drive_t *drive = cdi->handle;
 	struct cdrom_info *cd = drive->driver_data;
 	struct request_sense sense;
-	struct request req;
+	struct request *rq;
 	int ret;
 
-	ide_cd_init_rq(drive, &req);
-	req.cmd_type = REQ_TYPE_SPECIAL;
-	req.cmd_flags = REQ_QUIET;
-	ret = ide_do_drive_cmd(drive, &req, ide_wait);
-
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_flags = REQ_QUIET;
+	ret = blk_execute_rq(drive->queue, cd->disk, rq, 0);
+	blk_put_request(rq);
 	/*
 	 * A reset will unlock the door. If it was previously locked,
 	 * lock it again.
-- 
1.5.4.2

[PATCH v2 03/13] ide-disk: convert ide_do_drive_cmd path to use blk_execute_rq

[FUJITA Tomonori]

This converts the ide_do_drive_cmd path using ide_wait to use
blk_execute_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-disk.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index 8e08d08..c5f22ef 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -617,7 +617,8 @@ static void idedisk_prepare_flush(struct request_queue *q, struct request *rq)
  */
 static int set_multcount(ide_drive_t *drive, int arg)
 {
-	struct request rq;
+	struct request *rq;
+	int error;
 
 	if (arg < 0 || arg > drive->id->max_multsect)
 		return -EINVAL;
@@ -625,12 +626,13 @@ static int set_multcount(ide_drive_t *drive, int arg)
 	if (drive->special.b.set_multmode)
 		return -EBUSY;
 
-	ide_init_drive_cmd(&rq);
-	rq.cmd_type = REQ_TYPE_ATA_TASKFILE;
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
 
 	drive->mult_req = arg;
 	drive->special.b.set_multmode = 1;
-	(void)ide_do_drive_cmd(drive, &rq, ide_wait);
+	error = blk_execute_rq(drive->queue, NULL, rq, 0);
+	blk_put_request(rq);
 
 	return (drive->mult_count == arg) ? 0 : -EIO;
 }
-- 
1.5.4.2

[PATCH v2 04/13] ide-floppy: convert ide_do_drive_cmd path to use blk_execute_rq

[FUJITA Tomonori]

This converts the ide_do_drive_cmd path using ide_wait to use
blk_execute_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-floppy.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index f05fbc2..6bd0021 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -886,14 +886,16 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
 static int idefloppy_queue_pc_tail(ide_drive_t *drive, struct ide_atapi_pc *pc)
 {
 	struct ide_floppy_obj *floppy = drive->driver_data;
-	struct request rq;
+	struct request *rq;
+	int error;
 
-	ide_init_drive_cmd(&rq);
-	rq.buffer = (char *) pc;
-	rq.cmd_type = REQ_TYPE_SPECIAL;
-	rq.rq_disk = floppy->disk;
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->buffer = (char *) pc;
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	error = blk_execute_rq(drive->queue, floppy->disk, rq, 0);
+	blk_put_request(rq);
 
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
+	return error;
 }
 
 /*
-- 
1.5.4.2

[PATCH v2 05/13] ide-taskfile: convert ide_do_drive_cmd path to use blk_execute_rq

[FUJITA Tomonori]

This converts the ide_do_drive_cmd path using ide_wait to use
blk_execute_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-taskfile.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index 0c908ca..43e23ff 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -492,11 +492,12 @@ static ide_startstop_t pre_task_out_intr(ide_drive_t *drive, struct request *rq)
 
 int ide_raw_taskfile(ide_drive_t *drive, ide_task_t *task, u8 *buf, u16 nsect)
 {
-	struct request rq;
+	struct request *rq;
+	int error;
 
-	blk_rq_init(NULL, &rq);
-	rq.cmd_type = REQ_TYPE_ATA_TASKFILE;
-	rq.buffer = buf;
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
+	rq->buffer = buf;
 
 	/*
 	 * (ks) We transfer currently only whole sectors.
@@ -504,16 +505,19 @@ int ide_raw_taskfile(ide_drive_t *drive, ide_task_t *task, u8 *buf, u16 nsect)
 	 * if we would find a solution to transfer any size.
 	 * To support special commands like READ LONG.
 	 */
-	rq.hard_nr_sectors = rq.nr_sectors = nsect;
-	rq.hard_cur_sectors = rq.current_nr_sectors = nsect;
+	rq->hard_nr_sectors = rq->nr_sectors = nsect;
+	rq->hard_cur_sectors = rq->current_nr_sectors = nsect;
 
 	if (task->tf_flags & IDE_TFLAG_WRITE)
-		rq.cmd_flags |= REQ_RW;
+		rq->cmd_flags |= REQ_RW;
 
-	rq.special = task;
-	task->rq = &rq;
+	rq->special = task;
+	task->rq = rq;
 
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
+	error = blk_execute_rq(drive->queue, NULL, rq, 0);
+	blk_put_request(rq);
+
+	return error;
 }
 
 EXPORT_SYMBOL(ide_raw_taskfile);
@@ -739,12 +743,14 @@ int ide_cmd_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
 	struct hd_driveid *id = drive->id;
 
 	if (NULL == (void *) arg) {
-		struct request rq;
+		struct request *rq;
 
-		ide_init_drive_cmd(&rq);
-		rq.cmd_type = REQ_TYPE_ATA_TASKFILE;
+		rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+		rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
+		err = blk_execute_rq(drive->queue, NULL, rq, 0);
+		blk_put_request(rq);
 
-		return ide_do_drive_cmd(drive, &rq, ide_wait);
+		return err;
 	}
 
 	if (copy_from_user(args, (void __user *)arg, 4))
-- 
1.5.4.2

[PATCH v2 06/13] ide-tape: convert ide_do_drive_cmd path to use blk_execute_rq

[FUJITA Tomonori]

This converts the ide_do_drive_cmd path using ide_wait to use
blk_execute_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-tape.c |   41 ++++++++++++++++++++++++++---------------
 1 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 1e1f263..30b29dd 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1518,12 +1518,16 @@ static void idetape_create_test_unit_ready_cmd(struct ide_atapi_pc *pc)
 static int idetape_queue_pc_tail(ide_drive_t *drive, struct ide_atapi_pc *pc)
 {
 	struct ide_tape_obj *tape = drive->driver_data;
-	struct request rq;
+	struct request *rq;
+	int error;
 
-	idetape_init_rq(&rq, REQ_IDETAPE_PC1);
-	rq.buffer = (char *) pc;
-	rq.rq_disk = tape->disk;
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd[0] = REQ_IDETAPE_PC1;
+	rq->buffer = (char *)pc;
+	error = blk_execute_rq(drive->queue, tape->disk, rq, 0);
+	blk_put_request(rq);
+	return error;
 }
 
 static void idetape_create_load_unload_cmd(ide_drive_t *drive,
@@ -1700,26 +1704,33 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int blocks,
 				 struct idetape_bh *bh)
 {
 	idetape_tape_t *tape = drive->driver_data;
-	struct request rq;
+	struct request *rq;
+	int ret, errors;
 
 	debug_log(DBG_SENSE, "%s: cmd=%d\n", __func__, cmd);
 
-	idetape_init_rq(&rq, cmd);
-	rq.rq_disk = tape->disk;
-	rq.special = (void *)bh;
-	rq.sector = tape->first_frame;
-	rq.nr_sectors		= blocks;
-	rq.current_nr_sectors	= blocks;
-	(void) ide_do_drive_cmd(drive, &rq, ide_wait);
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd[0] = cmd;
+	rq->rq_disk = tape->disk;
+	rq->special = (void *)bh;
+	rq->sector = tape->first_frame;
+	rq->nr_sectors = blocks;
+	rq->current_nr_sectors = blocks;
+	blk_execute_rq(drive->queue, tape->disk, rq, 0);
+
+	errors = rq->errors;
+	ret = tape->blk_size * (blocks - rq->current_nr_sectors);
+	blk_put_request(rq);
 
 	if ((cmd & (REQ_IDETAPE_READ | REQ_IDETAPE_WRITE)) == 0)
 		return 0;
 
 	if (tape->merge_bh)
 		idetape_init_merge_buffer(tape);
-	if (rq.errors == IDETAPE_ERROR_GENERAL)
+	if (errors == IDETAPE_ERROR_GENERAL)
 		return -EIO;
-	return (tape->blk_size * (blocks-rq.current_nr_sectors));
+	return ret;
 }
 
 static void idetape_create_inquiry_cmd(struct ide_atapi_pc *pc)
-- 
1.5.4.2

[PATCH v2 07/13] ide: convert ide_do_drive_cmd path to use blk_execute_rq

[FUJITA Tomonori]

This converts the ide_do_drive_cmd path using ide_wait to use
blk_execute_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide.c |   38 ++++++++++++++++++++++----------------
 1 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index c758dcb..25e110e 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -498,7 +498,7 @@ out:
 
 int set_pio_mode(ide_drive_t *drive, int arg)
 {
-	struct request rq;
+	struct request *rq;
 	ide_hwif_t *hwif = drive->hwif;
 	const struct ide_port_ops *port_ops = hwif->port_ops;
 
@@ -512,12 +512,15 @@ int set_pio_mode(ide_drive_t *drive, int arg)
 	if (drive->special.b.set_tune)
 		return -EBUSY;
 
-	ide_init_drive_cmd(&rq);
-	rq.cmd_type = REQ_TYPE_ATA_TASKFILE;
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
 
 	drive->tune_req = (u8) arg;
 	drive->special.b.set_tune = 1;
-	(void) ide_do_drive_cmd(drive, &rq, ide_wait);
+
+	blk_execute_rq(drive->queue, NULL, rq, 0);
+	blk_put_request(rq);
+
 	return 0;
 }
 
@@ -555,7 +558,7 @@ static int generic_ide_suspend(struct device *dev, pm_message_t mesg)
 {
 	ide_drive_t *drive = dev->driver_data;
 	ide_hwif_t *hwif = HWIF(drive);
-	struct request rq;
+	struct request *rq;
 	struct request_pm_state rqpm;
 	ide_task_t args;
 	int ret;
@@ -564,18 +567,19 @@ static int generic_ide_suspend(struct device *dev, pm_message_t mesg)
 	if (!(drive->dn % 2))
 		ide_acpi_get_timing(hwif);
 
-	blk_rq_init(NULL, &rq);
 	memset(&rqpm, 0, sizeof(rqpm));
 	memset(&args, 0, sizeof(args));
-	rq.cmd_type = REQ_TYPE_PM_SUSPEND;
-	rq.special = &args;
-	rq.data = &rqpm;
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_PM_SUSPEND;
+	rq->special = &args;
+	rq->data = &rqpm;
 	rqpm.pm_step = ide_pm_state_start_suspend;
 	if (mesg.event == PM_EVENT_PRETHAW)
 		mesg.event = PM_EVENT_FREEZE;
 	rqpm.pm_state = mesg.event;
 
-	ret = ide_do_drive_cmd(drive, &rq, ide_wait);
+	ret = blk_execute_rq(drive->queue, NULL, rq, 0);
+	blk_put_request(rq);
 	/* only call ACPI _PS3 after both drivers are suspended */
 	if (!ret && (((drive->dn % 2) && hwif->drives[0].present
 		 && hwif->drives[1].present)
@@ -589,7 +593,7 @@ static int generic_ide_resume(struct device *dev)
 {
 	ide_drive_t *drive = dev->driver_data;
 	ide_hwif_t *hwif = HWIF(drive);
-	struct request rq;
+	struct request *rq;
 	struct request_pm_state rqpm;
 	ide_task_t args;
 	int err;
@@ -602,16 +606,18 @@ static int generic_ide_resume(struct device *dev)
 
 	ide_acpi_exec_tfs(drive);
 
-	blk_rq_init(NULL, &rq);
 	memset(&rqpm, 0, sizeof(rqpm));
 	memset(&args, 0, sizeof(args));
-	rq.cmd_type = REQ_TYPE_PM_RESUME;
-	rq.special = &args;
-	rq.data = &rqpm;
+	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq->cmd_type = REQ_TYPE_PM_RESUME;
+	rq->special = &args;
+	rq->data = &rqpm;
+	rq->cmd_flags |= REQ_PREEMPT;
 	rqpm.pm_step = ide_pm_state_start_resume;
 	rqpm.pm_state = PM_EVENT_ON;
 
-	err = ide_do_drive_cmd(drive, &rq, ide_head_wait);
+	err = blk_execute_rq(drive->queue, NULL, rq, 1);
+	blk_put_request(rq);
 
 	if (err == 0 && dev->driver) {
 		ide_driver_t *drv = to_ide_driver(dev->driver);
-- 
1.5.4.2

[PATCH v2 08/13] ide: remove ide_wait/head_wait path in ide_do_drive_cmd

[FUJITA Tomonori]

Now all the users of ide_do_drive_cmd using ide_wait/head_wait are
converted to use blk_execute_rq This removes the ide_wait/head_wait
path in ide_do_drive_cmd.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-io.c |   27 +++------------------------
 1 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 6965253..fae7c70 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -1584,26 +1584,14 @@ int ide_do_drive_cmd (ide_drive_t *drive, struct request *rq, ide_action_t actio
 {
 	unsigned long flags;
 	ide_hwgroup_t *hwgroup = HWGROUP(drive);
-	DECLARE_COMPLETION_ONSTACK(wait);
-	int where = ELEVATOR_INSERT_BACK, err;
-	int must_wait = (action == ide_wait || action == ide_head_wait);
+	int where = ELEVATOR_INSERT_BACK;
 
 	rq->errors = 0;
 
-	/*
-	 * we need to hold an extra reference to request for safe inspection
-	 * after completion
-	 */
-	if (must_wait) {
-		rq->ref_count++;
-		rq->end_io_data = &wait;
-		rq->end_io = blk_end_sync_rq;
-	}
-
 	spin_lock_irqsave(&ide_lock, flags);
 	if (action == ide_preempt)
 		hwgroup->rq = NULL;
-	if (action == ide_preempt || action == ide_head_wait) {
+	if (action == ide_preempt) {
 		where = ELEVATOR_INSERT_FRONT;
 		rq->cmd_flags |= REQ_PREEMPT;
 	}
@@ -1611,16 +1599,7 @@ int ide_do_drive_cmd (ide_drive_t *drive, struct request *rq, ide_action_t actio
 	ide_do_request(hwgroup, IDE_NO_IRQ);
 	spin_unlock_irqrestore(&ide_lock, flags);
 
-	err = 0;
-	if (must_wait) {
-		wait_for_completion(&wait);
-		if (rq->errors)
-			err = -EIO;
-
-		blk_put_request(rq);
-	}
-
-	return err;
+	return 0;
 }
 
 EXPORT_SYMBOL(ide_do_drive_cmd);
-- 
1.5.4.2

[PATCH v2 09/13] ide: remove ide_init_drive_cmd

[FUJITA Tomonori]

ide_init_drive_cmd just calls blk_rq_init. This converts the users of
ide_init_drive_cmd to use blk_rq_init directly and removes
ide_init_drive_cmd.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd.c     |    2 +-
 drivers/ide/ide-floppy.c |    2 +-
 drivers/ide/ide-io.c     |   17 -----------------
 drivers/scsi/ide-scsi.c  |    4 ++--
 include/linux/ide.h      |    2 --
 5 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index cb573b9..069fd6e 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -193,7 +193,7 @@ void ide_cd_init_rq(ide_drive_t *drive, struct request *rq)
 {
 	struct cdrom_info *cd = drive->driver_data;
 
-	ide_init_drive_cmd(rq);
+	blk_rq_init(NULL, rq);
 	rq->cmd_type = REQ_TYPE_ATA_PC;
 	rq->rq_disk = cd->disk;
 }
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 6bd0021..1199579 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -286,7 +286,7 @@ static void idefloppy_queue_pc_head(ide_drive_t *drive, struct ide_atapi_pc *pc,
 {
 	struct ide_floppy_obj *floppy = drive->driver_data;
 
-	ide_init_drive_cmd(rq);
+	blk_rq_init(NULL, rq);
 	rq->buffer = (char *) pc;
 	rq->cmd_type = REQ_TYPE_SPECIAL;
 	rq->rq_disk = floppy->disk;
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index fae7c70..a913b4b 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -1539,23 +1539,6 @@ irqreturn_t ide_intr (int irq, void *dev_id)
 }
 
 /**
- *	ide_init_drive_cmd	-	initialize a drive command request
- *	@rq: request object
- *
- *	Initialize a request before we fill it in and send it down to
- *	ide_do_drive_cmd. Commands must be set up by this function. Right
- *	now it doesn't do a lot, but if that changes abusers will have a
- *	nasty surprise.
- */
-
-void ide_init_drive_cmd (struct request *rq)
-{
-	blk_rq_init(NULL, rq);
-}
-
-EXPORT_SYMBOL(ide_init_drive_cmd);
-
-/**
  *	ide_do_drive_cmd	-	issue IDE special command
  *	@drive: device to issue command
  *	@rq: request to issue
diff --git a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
index 44d8d51..68a1714 100644
--- a/drivers/scsi/ide-scsi.c
+++ b/drivers/scsi/ide-scsi.c
@@ -228,7 +228,7 @@ static int idescsi_check_condition(ide_drive_t *drive,
 		kfree(pc);
 		return -ENOMEM;
 	}
-	ide_init_drive_cmd(rq);
+	blk_rq_init(NULL, rq);
 	rq->special = (char *) pc;
 	pc->rq = rq;
 	pc->buf = buf;
@@ -785,7 +785,7 @@ static int idescsi_queue (struct scsi_cmnd *cmd,
 		}
 	}
 
-	ide_init_drive_cmd (rq);
+	blk_rq_init(NULL, rq);
 	rq->special = (char *) pc;
 	rq->cmd_type = REQ_TYPE_SPECIAL;
 	spin_unlock_irq(host->host_lock);
diff --git a/include/linux/ide.h b/include/linux/ide.h
index b0135b0..fbf352d 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -842,8 +842,6 @@ int ide_wait_stat(ide_startstop_t *, ide_drive_t *, u8, u8, unsigned long);
 
 extern ide_startstop_t ide_do_reset (ide_drive_t *);
 
-extern void ide_init_drive_cmd (struct request *rq);
-
 /*
  * "action" parameter type for ide_do_drive_cmd() below.
  */
-- 
1.5.4.2

[PATCH v2 10/13] ide-cd: remove ide_cd_init_rq

[FUJITA Tomonori]

ide_cd_init_rq is not used by ide-cd_ioctl any more. Only
cdrom_queue_request_sense use it. This converts
cdrom_queue_request_sense to use blk_rq_init directly and removes
ide_cd_init_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd.c |   14 +++-----------
 drivers/ide/ide-cd.h |    1 -
 2 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 069fd6e..f1ee432 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -188,16 +188,6 @@ static void cdrom_analyze_sense_data(ide_drive_t *drive,
 	ide_cd_log_error(drive->name, failed_command, sense);
 }
 
-/* Initialize a ide-cd packet command request */
-void ide_cd_init_rq(ide_drive_t *drive, struct request *rq)
-{
-	struct cdrom_info *cd = drive->driver_data;
-
-	blk_rq_init(NULL, rq);
-	rq->cmd_type = REQ_TYPE_ATA_PC;
-	rq->rq_disk = cd->disk;
-}
-
 static void cdrom_queue_request_sense(ide_drive_t *drive, void *sense,
 				      struct request *failed_command)
 {
@@ -208,7 +198,9 @@ static void cdrom_queue_request_sense(ide_drive_t *drive, void *sense,
 		sense = &info->sense_data;
 
 	/* stuff the sense request in front of our current request */
-	ide_cd_init_rq(drive, rq);
+	blk_rq_init(NULL, rq);
+	rq->cmd_type = REQ_TYPE_ATA_PC;
+	rq->rq_disk = info->disk;
 
 	rq->data = sense;
 	rq->cmd[0] = GPCMD_REQUEST_SENSE;
diff --git a/drivers/ide/ide-cd.h b/drivers/ide/ide-cd.h
index 13918c5..a7971d7 100644
--- a/drivers/ide/ide-cd.h
+++ b/drivers/ide/ide-cd.h
@@ -143,7 +143,6 @@ struct cdrom_info {
 void ide_cd_log_error(const char *, struct request *, struct request_sense *);
 
 /* ide-cd.c functions used by ide-cd_ioctl.c */
-void ide_cd_init_rq(ide_drive_t *, struct request *);
 int ide_cd_queue_pc(ide_drive_t *, const unsigned char *, int, void *, unsigned,
 		    struct request_sense *, int, unsigned int);
 int ide_cd_read_toc(ide_drive_t *, struct request_sense *);
-- 
1.5.4.2

[PATCH v2 11/13] block: convert pd_special_command to use blk_execute_rq

[FUJITA Tomonori]

pd_special_command uses blk_put_request with struct request on the
stack. As a result, blk_put_request needs a hack to catch a NULL
request_queue.  This converts pd_special_command to use
blk_execute_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
 drivers/block/paride/pd.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index 570f3b7..5fdfa7c 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -712,19 +712,17 @@ static void do_pd_request(struct request_queue * q)
 static int pd_special_command(struct pd_unit *disk,
 		      enum action (*func)(struct pd_unit *disk))
 {
-	DECLARE_COMPLETION_ONSTACK(wait);
-	struct request rq;
+	struct request *rq;
 	int err = 0;
 
-	blk_rq_init(NULL, &rq);
-	rq.rq_disk = disk->gd;
-	rq.end_io_data = &wait;
-	rq.end_io = blk_end_sync_rq;
-	blk_insert_request(disk->gd->queue, &rq, 0, func);
-	wait_for_completion(&wait);
-	if (rq.errors)
-		err = -EIO;
-	blk_put_request(&rq);
+	rq = blk_get_request(disk->gd->queue, READ, __GFP_WAIT);
+
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->special = func;
+
+	err = blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
+
+	blk_put_request(rq);
 	return err;
 }
 
-- 
1.5.4.2

[PATCH v2 12/13] block: remove the checking for NULL queue in blk_put_request

[FUJITA Tomonori]

Some uses blk_put_request asymmetrically, that is, they uses it with
requests that not allocated by blk_get_request. As a result,
blk_put_request has a hack to catch a NULL request_queue. Now such
callers are fixed (they use blk_get_request properly). So we can
safely remove the hack in blk_put_request.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
 block/blk-core.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 5d09f8c..73337ee 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1043,15 +1043,9 @@ void blk_put_request(struct request *req)
 	unsigned long flags;
 	struct request_queue *q = req->q;
 
-	/*
-	 * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
-	 * following if (q) test.
-	 */
-	if (q) {
-		spin_lock_irqsave(q->queue_lock, flags);
-		__blk_put_request(q, req);
-		spin_unlock_irqrestore(q->queue_lock, flags);
-	}
+	spin_lock_irqsave(q->queue_lock, flags);
+	__blk_put_request(q, req);
+	spin_unlock_irqrestore(q->queue_lock, flags);
 }
 EXPORT_SYMBOL(blk_put_request);
 
-- 
1.5.4.2

[PATCH v2 13/13] block: unexport blk_end_sync_rq

[FUJITA Tomonori]

All the users of blk_end_sync_rq has gone (they are converted to use
blk_execute_rq). This unexports blk_end_sync_rq.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
 block/blk-exec.c       |    3 +--
 include/linux/blkdev.h |    1 -
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/block/blk-exec.c b/block/blk-exec.c
index 391dd62..f7213a3 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -18,7 +18,7 @@
  * @rq: request to complete
  * @error: end io status of the request
  */
-void blk_end_sync_rq(struct request *rq, int error)
+static void blk_end_sync_rq(struct request *rq, int error)
 {
 	struct completion *waiting = rq->end_io_data;
 
@@ -31,7 +31,6 @@ void blk_end_sync_rq(struct request *rq, int error)
 	 */
 	complete(waiting);
 }
-EXPORT_SYMBOL(blk_end_sync_rq);
 
 /**
  * blk_execute_rq_nowait - insert a request into queue for execution
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d2a1b71..1171abd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -623,7 +623,6 @@ extern void generic_make_request(struct bio *bio);
 extern void blk_rq_init(struct request_queue *q, struct request *rq);
 extern void blk_put_request(struct request *);
 extern void __blk_put_request(struct request_queue *, struct request *);
-extern void blk_end_sync_rq(struct request *rq, int error);
 extern struct request *blk_get_request(struct request_queue *, int, gfp_t);
 extern void blk_insert_request(struct request_queue *, struct request *, int, void *);
 extern void blk_requeue_request(struct request_queue *, struct request *);
-- 
1.5.4.2

Re: [PATCH v2 00/13] removing the on-stack struct request

[Borislav Petkov]

Hi,

On Thu, May 01, 2008 at 09:27:57PM +0900, FUJITA Tomonori wrote:
> This is an updated version of the patchset to clean up the asymmetry
> of blk_get/put_request usage:
> 
> http://marc.info/?l=linux-ide&m=120882410712466&w=2
> 
> This patchset removes the code calling blk_put_request against the
> requests that are not allocated via blk_get_request. They can use
> __GFP_WAIT allocation so we can easily convert them to
> use blk_get/put_request properly.
> 
> This patchset enables us to remove the following hack in
> blk_put_request:
> 
> /*
>  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
>  * following if (q) test.
>  */
> if (q) {
> 	spin_lock_irqsave(q->queue_lock, flags);
> 	__blk_put_request(q, req);
> 	spin_unlock_irqrestore(q->queue_lock, flags);
> }
> 
> The major changes are:
> 
> - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> the same thing that blk_execute_rq() does.

Are you sure about that? Not that I'm that familiar with the blk layer internals
but at a glance blk_execute_rq() calls blk_execute_rq_nowait() and in it, the
rq is added to the elevator and the device queue is being plugged (4th arg
to __elv_add_request(...,1)) and after the last returns the device is being
unplugged again: __generic_unplug_device(q). This way no rq's are getting merged
and it might hurt performance, IMHO.

-- 
Regards/Gruß,
    Boris.

Re: [PATCH v2 00/13] removing the on-stack struct request

[FUJITA Tomonori]

Hi, thanks for the comments,

On Fri, 2 May 2008 16:04:54 +0200
Borislav Petkov <petkovbb@googlemail.com> wrote:

> Hi,
> 
> On Thu, May 01, 2008 at 09:27:57PM +0900, FUJITA Tomonori wrote:
> > This is an updated version of the patchset to clean up the asymmetry
> > of blk_get/put_request usage:
> > 
> > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > 
> > This patchset removes the code calling blk_put_request against the
> > requests that are not allocated via blk_get_request. They can use
> > __GFP_WAIT allocation so we can easily convert them to
> > use blk_get/put_request properly.
> > 
> > This patchset enables us to remove the following hack in
> > blk_put_request:
> > 
> > /*
> >  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
> >  * following if (q) test.
> >  */
> > if (q) {
> > 	spin_lock_irqsave(q->queue_lock, flags);
> > 	__blk_put_request(q, req);
> > 	spin_unlock_irqrestore(q->queue_lock, flags);
> > }
> > 
> > The major changes are:
> > 
> > - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> > the same thing that blk_execute_rq() does.
> 
> Are you sure about that? Not that I'm that familiar with the blk layer internals
> but at a glance blk_execute_rq() calls blk_execute_rq_nowait() and in it, the
> rq is added to the elevator and the device queue is being plugged (4th arg
> to __elv_add_request(...,1)) and after the last returns the device is being
> unplugged again: __generic_unplug_device(q). This way no rq's are getting merged
> and it might hurt performance, IMHO.

Yes, no requests are merged, but

1. the ide_wait/head_wait path in ide_do_drive_cmd() doesn't handle any
mergeable requests. All the requests are for kinda management. We like
to execute such as soon as possible.

2. I don't think that ide_do_drive_cmd merges requests. In case of
ELEVATOR_INSERT_FRONT, it calls ide_do_request right after
elv_insert. ide_do_request takes a request from the queue and executes
it? In case of ELEVATOR_INSERT_BACK, elv_insert calls q->request_fn,
that is, ide_do_request is called.

Re: [PATCH v2 00/13] removing the on-stack struct request

[Bartlomiej Zolnierkiewicz]

Hi,

On Thursday 01 May 2008, FUJITA Tomonori wrote:
> This is an updated version of the patchset to clean up the asymmetry
> of blk_get/put_request usage:
> 
> http://marc.info/?l=linux-ide&m=120882410712466&w=2
> 
> This patchset removes the code calling blk_put_request against the
> requests that are not allocated via blk_get_request. They can use
> __GFP_WAIT allocation so we can easily convert them to
> use blk_get/put_request properly.
> 
> This patchset enables us to remove the following hack in
> blk_put_request:
> 
> /*
>  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
>  * following if (q) test.
>  */
> if (q) {
> 	spin_lock_irqsave(q->queue_lock, flags);
> 	__blk_put_request(q, req);
> 	spin_unlock_irqrestore(q->queue_lock, flags);
> }
> 
> The major changes are:
> 
> - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> the same thing that blk_execute_rq() does. So let's simply use
> blk_execute_rq(). The nice side effect is that I unexport
> blk_end_sync_rq() since I converted all the users.
> 
> - I drop the unnecessary patch to make ide_do_drive_cmd return
> rq->errors:
> 
> http://marc.info/?l=linux-ide&m=120882410612456&w=2
> 
> #1-10 are for the ide subsystem and #11-13 for the block layer. This
> is against the lastest Linus tree.

Thanks.

I applied patches #1-10 after making ide_do_drive_cmd() more similar to
blk_execute_rq() (to ease the conversion - it is really better to handle
subtle changes first). [ I'll send these pre-patches in separate mails. ]

However I'm still not quite sure about following change in patch #1:

@@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
...
-	cgc->stat = ide_cd_queue_pc(drive, &req);
+	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
+				    cgc->data_direction == CGC_DATA_WRITE,
+				    cgc->buffer, cgc->buflen,
+				    cgc->sense, cgc->timeout, flags);
 	if (!cgc->stat)
-		cgc->buflen -= req.data_len;
+		cgc->buflen = 0;
...

IIRC rq->data_len may be modified while the request is processed?

[ The only other (minor) complaint is that patches #9-10 should be at the
  beginning of the patch series but since no patches contained problems it
  wouldn't be wise to ask respin just for that (and I'm too lazy to review
  it all again ;-). ]

Thanks,
Bart

Re: [PATCH v2 00/13] removing the on-stack struct request

[FUJITA Tomonori]

On Sat, 3 May 2008 15:10:02 +0200
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> 
> Hi,
> 
> On Thursday 01 May 2008, FUJITA Tomonori wrote:
> > This is an updated version of the patchset to clean up the asymmetry
> > of blk_get/put_request usage:
> > 
> > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > 
> > This patchset removes the code calling blk_put_request against the
> > requests that are not allocated via blk_get_request. They can use
> > __GFP_WAIT allocation so we can easily convert them to
> > use blk_get/put_request properly.
> > 
> > This patchset enables us to remove the following hack in
> > blk_put_request:
> > 
> > /*
> >  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
> >  * following if (q) test.
> >  */
> > if (q) {
> > 	spin_lock_irqsave(q->queue_lock, flags);
> > 	__blk_put_request(q, req);
> > 	spin_unlock_irqrestore(q->queue_lock, flags);
> > }
> > 
> > The major changes are:
> > 
> > - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> > the same thing that blk_execute_rq() does. So let's simply use
> > blk_execute_rq(). The nice side effect is that I unexport
> > blk_end_sync_rq() since I converted all the users.
> > 
> > - I drop the unnecessary patch to make ide_do_drive_cmd return
> > rq->errors:
> > 
> > http://marc.info/?l=linux-ide&m=120882410612456&w=2
> > 
> > #1-10 are for the ide subsystem and #11-13 for the block layer. This
> > is against the lastest Linus tree.
> 
> Thanks.
> 
> I applied patches #1-10 after making ide_do_drive_cmd() more similar to
> blk_execute_rq() (to ease the conversion - it is really better to handle
> subtle changes first). [ I'll send these pre-patches in separate mails. ]

Thanks for applying the patches.


> However I'm still not quite sure about following change in patch #1:
> 
> @@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
> ...
> -	cgc->stat = ide_cd_queue_pc(drive, &req);
> +	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
> +				    cgc->data_direction == CGC_DATA_WRITE,
> +				    cgc->buffer, cgc->buflen,
> +				    cgc->sense, cgc->timeout, flags);
>  	if (!cgc->stat)
> -		cgc->buflen -= req.data_len;
> +		cgc->buflen = 0;
> ...
> 
> IIRC rq->data_len may be modified while the request is processed?

I think that rq->data_len is modified only when a request is
successful but I might overlook or misunderstand something. How about
the attached patch (against the latest your quilt patchset)? It's a
bit hacky but it should work as before.


> [ The only other (minor) complaint is that patches #9-10 should be at the
>   beginning of the patch series but since no patches contained problems it
>   wouldn't be wise to ask respin just for that (and I'm too lazy to review
>   it all again ;-). ]

Thanks.

If you are ok with the attached patch, I'm happy to incorporate it
into the patchset and send a new version as you like.


==
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: ide: let ide_cd_queue_pc return rq->data_len

ide_cdrom_packet needs rq->data_len after the completion so this patch
makes ide_cd_queue_pc return rq->data_len.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 7c5ab78..ac542ff 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -832,7 +832,7 @@ static void ide_cd_request_sense_fixup(struct request *rq)
 }
 
 int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char *cmd,
-		    int write, void *buffer, unsigned bufflen,
+		    int write, void *buffer, unsigned *bufflen,
 		    struct request_sense *sense, int timeout,
 		    unsigned int cmd_flags)
 {
@@ -855,12 +855,17 @@ int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char *cmd,
 		rq->cmd_type = REQ_TYPE_ATA_PC;
 		rq->sense = sense;
 		rq->cmd_flags |= cmd_flags;
-		rq->data = buffer;
-		rq->data_len = bufflen;
 		rq->timeout = timeout;
+		if (buffer) {
+			rq->data = buffer;
+			rq->data_len = *bufflen;
+		}
 
 		error = blk_execute_rq(drive->queue, info->disk, rq, 0);
 
+		if (buffer)
+			*bufflen = rq->data_len;
+
 		flags = rq->cmd_flags;
 		blk_put_request(rq);
 
@@ -1303,12 +1308,13 @@ static int cdrom_read_capacity(ide_drive_t *drive, unsigned long *capacity,
 
 	int stat;
 	unsigned char cmd[BLK_MAX_CDB];
+	unsigned len = sizeof(capbuf);
 
 	memset(cmd, 0, BLK_MAX_CDB);
 	cmd[0] = GPCMD_READ_CDVD_CAPACITY;
 
-	stat = ide_cd_queue_pc(drive, cmd, 0, &capbuf, sizeof(capbuf),
-			       sense, 0, REQ_QUIET);
+	stat = ide_cd_queue_pc(drive, cmd, 0, &capbuf, &len, sense, 0,
+			       REQ_QUIET);
 	if (stat == 0) {
 		*capacity = 1 + be32_to_cpu(capbuf.lba);
 		*sectors_per_frame =
@@ -1335,7 +1341,7 @@ static int cdrom_read_tocentry(ide_drive_t *drive, int trackno, int msf_flag,
 	if (msf_flag)
 		cmd[1] = 2;
 
-	return ide_cd_queue_pc(drive, cmd, 0, buf, buflen, sense, 0, REQ_QUIET);
+	return ide_cd_queue_pc(drive, cmd, 0, buf, &buflen, sense, 0, REQ_QUIET);
 }
 
 /* Try to read the entire TOC for the disk into our internal buffer. */
diff --git a/drivers/ide/ide-cd.h b/drivers/ide/ide-cd.h
index a7971d7..94f5e4e 100644
--- a/drivers/ide/ide-cd.h
+++ b/drivers/ide/ide-cd.h
@@ -143,7 +143,7 @@ struct cdrom_info {
 void ide_cd_log_error(const char *, struct request *, struct request_sense *);
 
 /* ide-cd.c functions used by ide-cd_ioctl.c */
-int ide_cd_queue_pc(ide_drive_t *, const unsigned char *, int, void *, unsigned,
+int ide_cd_queue_pc(ide_drive_t *, const unsigned char *, int, void *, unsigned *,
 		    struct request_sense *, int, unsigned int);
 int ide_cd_read_toc(ide_drive_t *, struct request_sense *);
 int ide_cdrom_get_capabilities(ide_drive_t *, u8 *);
diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
index 4571b4f..24d002a 100644
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -270,6 +270,7 @@ int ide_cdrom_get_mcn(struct cdrom_device_info *cdi,
 	int stat, mcnlen;
 	char buf[24];
 	unsigned char cmd[BLK_MAX_CDB];
+	unsigned len = sizeof(buf);
 
 	memset(cmd, 0, BLK_MAX_CDB);
 
@@ -277,9 +278,9 @@ int ide_cdrom_get_mcn(struct cdrom_device_info *cdi,
 	cmd[1] = 2;		/* MSF addressing */
 	cmd[2] = 0x40;	/* request subQ data */
 	cmd[3] = 2;		/* format */
-	cmd[8] = sizeof(buf);
+	cmd[8] = len;
 
-	stat = ide_cd_queue_pc(drive, cmd, 0, buf, sizeof(buf), NULL, 0, 0);
+	stat = ide_cd_queue_pc(drive, cmd, 0, buf, &len, NULL, 0, 0);
 	if (stat)
 		return stat;
 
@@ -445,6 +446,7 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
 {
 	ide_drive_t *drive = cdi->handle;
 	unsigned int flags = 0;
+	unsigned len = cgc->buflen;
 
 	if (cgc->timeout <= 0)
 		cgc->timeout = ATAPI_WAIT_PC;
@@ -464,9 +466,9 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
 
 	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
 				    cgc->data_direction == CGC_DATA_WRITE,
-				    cgc->buffer, cgc->buflen,
+				    cgc->buffer, &len,
 				    cgc->sense, cgc->timeout, flags);
 	if (!cgc->stat)
-		cgc->buflen = 0;
+		cgc->buflen -= len;
 	return cgc->stat;
 }

Re: [PATCH v2 00/13] removing the on-stack struct request

[Bartlomiej Zolnierkiewicz]

On Monday 05 May 2008, FUJITA Tomonori wrote:
> On Sat, 3 May 2008 15:10:02 +0200
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
> > 
> > Hi,
> > 
> > On Thursday 01 May 2008, FUJITA Tomonori wrote:
> > > This is an updated version of the patchset to clean up the asymmetry
> > > of blk_get/put_request usage:
> > > 
> > > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > > 
> > > This patchset removes the code calling blk_put_request against the
> > > requests that are not allocated via blk_get_request. They can use
> > > __GFP_WAIT allocation so we can easily convert them to
> > > use blk_get/put_request properly.
> > > 
> > > This patchset enables us to remove the following hack in
> > > blk_put_request:
> > > 
> > > /*
> > >  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
> > >  * following if (q) test.
> > >  */
> > > if (q) {
> > > 	spin_lock_irqsave(q->queue_lock, flags);
> > > 	__blk_put_request(q, req);
> > > 	spin_unlock_irqrestore(q->queue_lock, flags);
> > > }
> > > 
> > > The major changes are:
> > > 
> > > - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> > > the same thing that blk_execute_rq() does. So let's simply use
> > > blk_execute_rq(). The nice side effect is that I unexport
> > > blk_end_sync_rq() since I converted all the users.
> > > 
> > > - I drop the unnecessary patch to make ide_do_drive_cmd return
> > > rq->errors:
> > > 
> > > http://marc.info/?l=linux-ide&m=120882410612456&w=2
> > > 
> > > #1-10 are for the ide subsystem and #11-13 for the block layer. This
> > > is against the lastest Linus tree.
> > 
> > Thanks.
> > 
> > I applied patches #1-10 after making ide_do_drive_cmd() more similar to
> > blk_execute_rq() (to ease the conversion - it is really better to handle
> > subtle changes first). [ I'll send these pre-patches in separate mails. ]
> 
> Thanks for applying the patches.
> 
> 
> > However I'm still not quite sure about following change in patch #1:
> > 
> > @@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
> > ...
> > -	cgc->stat = ide_cd_queue_pc(drive, &req);
> > +	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
> > +				    cgc->data_direction == CGC_DATA_WRITE,
> > +				    cgc->buffer, cgc->buflen,
> > +				    cgc->sense, cgc->timeout, flags);
> >  	if (!cgc->stat)
> > -		cgc->buflen -= req.data_len;
> > +		cgc->buflen = 0;
> > ...
> > 
> > IIRC rq->data_len may be modified while the request is processed?
> 
> I think that rq->data_len is modified only when a request is
> successful but I might overlook or misunderstand something. How about
> the attached patch (against the latest your quilt patchset)? It's a
> bit hacky but it should work as before.
> 
> 
> > [ The only other (minor) complaint is that patches #9-10 should be at the
> >   beginning of the patch series but since no patches contained problems it
> >   wouldn't be wise to ask respin just for that (and I'm too lazy to review
> >   it all again ;-). ]
> 
> Thanks.
> 
> If you are ok with the attached patch, I'm happy to incorporate it
> into the patchset and send a new version as you like.
> 
> 
> ==
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: ide: let ide_cd_queue_pc return rq->data_len
> 
> ide_cdrom_packet needs rq->data_len after the completion so this patch
> makes ide_cd_queue_pc return rq->data_len.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

Thanks!  I just integrated this change with patch #1 so there is no need
for you to repost the whole patchset.

WRT patches #11-13: since they are quite straightforward I wonder whether
it might make sense to push them through IDE tree (they depend on #1-10)?

Jens, if you are fine with it I'll add your SOB and add them to the queue.

Thanks,
Bart

Re: [PATCH v2 00/13] removing the on-stack struct request

[Jens Axboe]

On Mon, May 05 2008, Bartlomiej Zolnierkiewicz wrote:
> On Monday 05 May 2008, FUJITA Tomonori wrote:
> > On Sat, 3 May 2008 15:10:02 +0200
> > Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> > 
> > > 
> > > Hi,
> > > 
> > > On Thursday 01 May 2008, FUJITA Tomonori wrote:
> > > > This is an updated version of the patchset to clean up the asymmetry
> > > > of blk_get/put_request usage:
> > > > 
> > > > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > > > 
> > > > This patchset removes the code calling blk_put_request against the
> > > > requests that are not allocated via blk_get_request. They can use
> > > > __GFP_WAIT allocation so we can easily convert them to
> > > > use blk_get/put_request properly.
> > > > 
> > > > This patchset enables us to remove the following hack in
> > > > blk_put_request:
> > > > 
> > > > /*
> > > >  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
> > > >  * following if (q) test.
> > > >  */
> > > > if (q) {
> > > > 	spin_lock_irqsave(q->queue_lock, flags);
> > > > 	__blk_put_request(q, req);
> > > > 	spin_unlock_irqrestore(q->queue_lock, flags);
> > > > }
> > > > 
> > > > The major changes are:
> > > > 
> > > > - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> > > > the same thing that blk_execute_rq() does. So let's simply use
> > > > blk_execute_rq(). The nice side effect is that I unexport
> > > > blk_end_sync_rq() since I converted all the users.
> > > > 
> > > > - I drop the unnecessary patch to make ide_do_drive_cmd return
> > > > rq->errors:
> > > > 
> > > > http://marc.info/?l=linux-ide&m=120882410612456&w=2
> > > > 
> > > > #1-10 are for the ide subsystem and #11-13 for the block layer. This
> > > > is against the lastest Linus tree.
> > > 
> > > Thanks.
> > > 
> > > I applied patches #1-10 after making ide_do_drive_cmd() more similar to
> > > blk_execute_rq() (to ease the conversion - it is really better to handle
> > > subtle changes first). [ I'll send these pre-patches in separate mails. ]
> > 
> > Thanks for applying the patches.
> > 
> > 
> > > However I'm still not quite sure about following change in patch #1:
> > > 
> > > @@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
> > > ...
> > > -	cgc->stat = ide_cd_queue_pc(drive, &req);
> > > +	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
> > > +				    cgc->data_direction == CGC_DATA_WRITE,
> > > +				    cgc->buffer, cgc->buflen,
> > > +				    cgc->sense, cgc->timeout, flags);
> > >  	if (!cgc->stat)
> > > -		cgc->buflen -= req.data_len;
> > > +		cgc->buflen = 0;
> > > ...
> > > 
> > > IIRC rq->data_len may be modified while the request is processed?
> > 
> > I think that rq->data_len is modified only when a request is
> > successful but I might overlook or misunderstand something. How about
> > the attached patch (against the latest your quilt patchset)? It's a
> > bit hacky but it should work as before.
> > 
> > 
> > > [ The only other (minor) complaint is that patches #9-10 should be at the
> > >   beginning of the patch series but since no patches contained problems it
> > >   wouldn't be wise to ask respin just for that (and I'm too lazy to review
> > >   it all again ;-). ]
> > 
> > Thanks.
> > 
> > If you are ok with the attached patch, I'm happy to incorporate it
> > into the patchset and send a new version as you like.
> > 
> > 
> > ==
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: ide: let ide_cd_queue_pc return rq->data_len
> > 
> > ide_cdrom_packet needs rq->data_len after the completion so this patch
> > makes ide_cd_queue_pc return rq->data_len.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> 
> Thanks!  I just integrated this change with patch #1 so there is no need
> for you to repost the whole patchset.
> 
> WRT patches #11-13: since they are quite straightforward I wonder whether
> it might make sense to push them through IDE tree (they depend on #1-10)?
> 
> Jens, if you are fine with it I'll add your SOB and add them to the queue.

Since they are so small, not an issue with me. You may add my
Signed-off-by, I'm glad that we are (finally) getting rid of the
on-stack requests.

-- 
Jens Axboe

Re: [PATCH v2 00/13] removing the on-stack struct request

[FUJITA Tomonori]

On Mon, 5 May 2008 21:04:09 +0200
Jens Axboe <jens.axboe@oracle.com> wrote:

> On Mon, May 05 2008, Bartlomiej Zolnierkiewicz wrote:
> > On Monday 05 May 2008, FUJITA Tomonori wrote:
> > > On Sat, 3 May 2008 15:10:02 +0200
> > > Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> > > 
> > > > 
> > > > Hi,
> > > > 
> > > > On Thursday 01 May 2008, FUJITA Tomonori wrote:
> > > > > This is an updated version of the patchset to clean up the asymmetry
> > > > > of blk_get/put_request usage:
> > > > > 
> > > > > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > > > > 
> > > > > This patchset removes the code calling blk_put_request against the
> > > > > requests that are not allocated via blk_get_request. They can use
> > > > > __GFP_WAIT allocation so we can easily convert them to
> > > > > use blk_get/put_request properly.
> > > > > 
> > > > > This patchset enables us to remove the following hack in
> > > > > blk_put_request:
> > > > > 
> > > > > /*
> > > > >  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
> > > > >  * following if (q) test.
> > > > >  */
> > > > > if (q) {
> > > > > 	spin_lock_irqsave(q->queue_lock, flags);
> > > > > 	__blk_put_request(q, req);
> > > > > 	spin_unlock_irqrestore(q->queue_lock, flags);
> > > > > }
> > > > > 
> > > > > The major changes are:
> > > > > 
> > > > > - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> > > > > the same thing that blk_execute_rq() does. So let's simply use
> > > > > blk_execute_rq(). The nice side effect is that I unexport
> > > > > blk_end_sync_rq() since I converted all the users.
> > > > > 
> > > > > - I drop the unnecessary patch to make ide_do_drive_cmd return
> > > > > rq->errors:
> > > > > 
> > > > > http://marc.info/?l=linux-ide&m=120882410612456&w=2
> > > > > 
> > > > > #1-10 are for the ide subsystem and #11-13 for the block layer. This
> > > > > is against the lastest Linus tree.
> > > > 
> > > > Thanks.
> > > > 
> > > > I applied patches #1-10 after making ide_do_drive_cmd() more similar to
> > > > blk_execute_rq() (to ease the conversion - it is really better to handle
> > > > subtle changes first). [ I'll send these pre-patches in separate mails. ]
> > > 
> > > Thanks for applying the patches.
> > > 
> > > 
> > > > However I'm still not quite sure about following change in patch #1:
> > > > 
> > > > @@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
> > > > ...
> > > > -	cgc->stat = ide_cd_queue_pc(drive, &req);
> > > > +	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
> > > > +				    cgc->data_direction == CGC_DATA_WRITE,
> > > > +				    cgc->buffer, cgc->buflen,
> > > > +				    cgc->sense, cgc->timeout, flags);
> > > >  	if (!cgc->stat)
> > > > -		cgc->buflen -= req.data_len;
> > > > +		cgc->buflen = 0;
> > > > ...
> > > > 
> > > > IIRC rq->data_len may be modified while the request is processed?
> > > 
> > > I think that rq->data_len is modified only when a request is
> > > successful but I might overlook or misunderstand something. How about
> > > the attached patch (against the latest your quilt patchset)? It's a
> > > bit hacky but it should work as before.
> > > 
> > > 
> > > > [ The only other (minor) complaint is that patches #9-10 should be at the
> > > >   beginning of the patch series but since no patches contained problems it
> > > >   wouldn't be wise to ask respin just for that (and I'm too lazy to review
> > > >   it all again ;-). ]
> > > 
> > > Thanks.
> > > 
> > > If you are ok with the attached patch, I'm happy to incorporate it
> > > into the patchset and send a new version as you like.
> > > 
> > > 
> > > ==
> > > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > Subject: ide: let ide_cd_queue_pc return rq->data_len
> > > 
> > > ide_cdrom_packet needs rq->data_len after the completion so this patch
> > > makes ide_cd_queue_pc return rq->data_len.
> > > 
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > 
> > Thanks!  I just integrated this change with patch #1 so there is no need
> > for you to repost the whole patchset.

Thanks a lot!


> > WRT patches #11-13: since they are quite straightforward I wonder whether
> > it might make sense to push them through IDE tree (they depend on #1-10)?
> > 
> > Jens, if you are fine with it I'll add your SOB and add them to the queue.
> 
> Since they are so small, not an issue with me. You may add my
> Signed-off-by, I'm glad that we are (finally) getting rid of the
> on-stack requests.

Yeah, the remaining on-stack requests are used in the paths that we
can't fail to allocate the requests (e.g. from interrupt
context). They are legitimate, as discussed before. We got rid of the
asymmetry of blk_get_request and blk_put_request usage in IDE and the
hack in blk_put_request so I'm glad too.

Thanks,

Re: [PATCH v2 00/13] removing the on-stack struct request

[Bartlomiej Zolnierkiewicz]

On Tuesday 06 May 2008, FUJITA Tomonori wrote:
> On Mon, 5 May 2008 21:04:09 +0200
> Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > On Mon, May 05 2008, Bartlomiej Zolnierkiewicz wrote:
> > > On Monday 05 May 2008, FUJITA Tomonori wrote:
> > > > On Sat, 3 May 2008 15:10:02 +0200
> > > > Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> > > > 
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > On Thursday 01 May 2008, FUJITA Tomonori wrote:
> > > > > > This is an updated version of the patchset to clean up the asymmetry
> > > > > > of blk_get/put_request usage:
> > > > > > 
> > > > > > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > > > > > 
> > > > > > This patchset removes the code calling blk_put_request against the
> > > > > > requests that are not allocated via blk_get_request. They can use
> > > > > > __GFP_WAIT allocation so we can easily convert them to
> > > > > > use blk_get/put_request properly.
> > > > > > 
> > > > > > This patchset enables us to remove the following hack in
> > > > > > blk_put_request:
> > > > > > 
> > > > > > /*
> > > > > >  * Gee, IDE calls in w/ NULL q.  Fix IDE and remove the
> > > > > >  * following if (q) test.
> > > > > >  */
> > > > > > if (q) {
> > > > > > 	spin_lock_irqsave(q->queue_lock, flags);
> > > > > > 	__blk_put_request(q, req);
> > > > > > 	spin_unlock_irqrestore(q->queue_lock, flags);
> > > > > > }
> > > > > > 
> > > > > > The major changes are:
> > > > > > 
> > > > > > - I remove the ide_wait/head_wait path in ide_do_drive_cmd(). It does
> > > > > > the same thing that blk_execute_rq() does. So let's simply use
> > > > > > blk_execute_rq(). The nice side effect is that I unexport
> > > > > > blk_end_sync_rq() since I converted all the users.
> > > > > > 
> > > > > > - I drop the unnecessary patch to make ide_do_drive_cmd return
> > > > > > rq->errors:
> > > > > > 
> > > > > > http://marc.info/?l=linux-ide&m=120882410612456&w=2
> > > > > > 
> > > > > > #1-10 are for the ide subsystem and #11-13 for the block layer. This
> > > > > > is against the lastest Linus tree.
> > > > > 
> > > > > Thanks.
> > > > > 
> > > > > I applied patches #1-10 after making ide_do_drive_cmd() more similar to
> > > > > blk_execute_rq() (to ease the conversion - it is really better to handle
> > > > > subtle changes first). [ I'll send these pre-patches in separate mails. ]
> > > > 
> > > > Thanks for applying the patches.
> > > > 
> > > > 
> > > > > However I'm still not quite sure about following change in patch #1:
> > > > > 
> > > > > @@ -456,24 +452,21 @@ int ide_cdrom_packet(struct cdrom_device_info *cdi,
> > > > > ...
> > > > > -	cgc->stat = ide_cd_queue_pc(drive, &req);
> > > > > +	cgc->stat = ide_cd_queue_pc(drive, cgc->cmd,
> > > > > +				    cgc->data_direction == CGC_DATA_WRITE,
> > > > > +				    cgc->buffer, cgc->buflen,
> > > > > +				    cgc->sense, cgc->timeout, flags);
> > > > >  	if (!cgc->stat)
> > > > > -		cgc->buflen -= req.data_len;
> > > > > +		cgc->buflen = 0;
> > > > > ...
> > > > > 
> > > > > IIRC rq->data_len may be modified while the request is processed?
> > > > 
> > > > I think that rq->data_len is modified only when a request is
> > > > successful but I might overlook or misunderstand something. How about
> > > > the attached patch (against the latest your quilt patchset)? It's a
> > > > bit hacky but it should work as before.
> > > > 
> > > > 
> > > > > [ The only other (minor) complaint is that patches #9-10 should be at the
> > > > >   beginning of the patch series but since no patches contained problems it
> > > > >   wouldn't be wise to ask respin just for that (and I'm too lazy to review
> > > > >   it all again ;-). ]
> > > > 
> > > > Thanks.
> > > > 
> > > > If you are ok with the attached patch, I'm happy to incorporate it
> > > > into the patchset and send a new version as you like.
> > > > 
> > > > 
> > > > ==
> > > > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > > Subject: ide: let ide_cd_queue_pc return rq->data_len
> > > > 
> > > > ide_cdrom_packet needs rq->data_len after the completion so this patch
> > > > makes ide_cd_queue_pc return rq->data_len.
> > > > 
> > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > 
> > > Thanks!  I just integrated this change with patch #1 so there is no need
> > > for you to repost the whole patchset.
> 
> Thanks a lot!
> 
> 
> > > WRT patches #11-13: since they are quite straightforward I wonder whether
> > > it might make sense to push them through IDE tree (they depend on #1-10)?
> > > 
> > > Jens, if you are fine with it I'll add your SOB and add them to the queue.
> > 
> > Since they are so small, not an issue with me. You may add my
> > Signed-off-by, I'm glad that we are (finally) getting rid of the
> > on-stack requests.

Done.

> Yeah, the remaining on-stack requests are used in the paths that we
> can't fail to allocate the requests (e.g. from interrupt
> context). They are legitimate, as discussed before. We got rid of the
> asymmetry of blk_get_request and blk_put_request usage in IDE and the
> hack in blk_put_request so I'm glad too.

Count me as another happy camper... :)

[ Now the only thing left to do is to deal with private struct request
  stacks in ATAPI device drivers, having just a single struct request
  (for REQUEST SENSE command) should be enough... ]

Thanks,
Bart