[PATCH 0/6] add large command support to the block layer

[PATCH 0/6] add large command support to the block layer

[FUJITA Tomonori]

This is an updated patchset for large command support to the block
layer:

http://marc.info/?l=linux-scsi&m=120817127118449&w=2

We rarely handle large commands. So for optimization, a struct request
still has a static array for a command. rq_init sets rq->cmd pointer
to the static array. In short, rq_init() does

rq->cmd = rq->__cmd;

So we can access to rq->cmd and rq->cmd_len as before.

This change requires everyone to initialize the request in a proper
way (that is, just doing a memset() will not work). Now we have
rq_init() that works for any path so this patchset can cleanly convert
users of requests on the stack or kmalloced requests to use it (the
previous patchset does it in a hacky way):

http://marc.info/?l=linux-scsi&m=120911792725876&w=2

This patchset is against Jens' for-linus branch.

#1-#4 patches can be applied via Jens' tree now. #5 patch is for
IDE. It cleanly can be applied to both Bart's latest quilt tree and
Jens' tree though Bart's quilt tree has some pending IDE patches. #4
patch depends on #4. #6 patch depends on #1-#5.

I guess that the easiest way to apply this patchset would be:

1. Pushing Bart's quilt tree to mainline.
2. Rebasing Jens' tree to mainline.
3. Pushing this patchset via Jens' tree.

Jens and Bart, let me know if I can do something to make the process
easier.

Bart, I will try to push the patchset to remove the requests on the
stack for 2.6.27:

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

[PATCH 2/6] block: no need to initialize rq->cmd with blk_get_request

[FUJITA Tomonori]

blk_get_request initializes rq->cmd (rq_init does) so the users don't
need to do that.

The purpose of this patch is to remove sizeof(rq->cmd) and &rq->cmd,
as a preparation for large command support, which changes rq->cmd from
the static array to a pointer. sizeof(rq->cmd) will not make sense and
&rq->cmd won't work.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
 block/scsi_ioctl.c          |    3 ---
 drivers/block/pktcdvd.c     |    2 --
 drivers/cdrom/cdrom.c       |    1 -
 drivers/md/dm-emc.c         |    2 --
 drivers/md/dm-mpath-hp-sw.c |    1 -
 drivers/md/dm-mpath-rdac.c  |    1 -
 6 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index a2c3a93..ffa3720 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -217,8 +217,6 @@ EXPORT_SYMBOL_GPL(blk_verify_command);
 static int blk_fill_sghdr_rq(struct request_queue *q, struct request *rq,
 			     struct sg_io_hdr *hdr, int has_write_perm)
 {
-	memset(rq->cmd, 0, BLK_MAX_CDB); /* ATAPI hates garbage after CDB */
-
 	if (copy_from_user(rq->cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
 	if (blk_verify_command(rq->cmd, has_write_perm))
@@ -531,7 +529,6 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk,
 	rq->data_len = 0;
 	rq->extra_len = 0;
 	rq->timeout = BLK_DEFAULT_SG_TIMEOUT;
-	memset(rq->cmd, 0, sizeof(rq->cmd));
 	rq->cmd[0] = cmd;
 	rq->cmd[4] = data;
 	rq->cmd_len = 6;
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 18feb1c..3b806c9 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -776,8 +776,6 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
 
 	rq->cmd_len = COMMAND_SIZE(cgc->cmd[0]);
 	memcpy(rq->cmd, cgc->cmd, CDROM_PACKET_SIZE);
-	if (sizeof(rq->cmd) > CDROM_PACKET_SIZE)
-		memset(rq->cmd + CDROM_PACKET_SIZE, 0, sizeof(rq->cmd) - CDROM_PACKET_SIZE);
 
 	rq->timeout = 60*HZ;
 	rq->cmd_type = REQ_TYPE_BLOCK_PC;
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index ac38290..69f26eb 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -2194,7 +2194,6 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
 		if (ret)
 			break;
 
-		memset(rq->cmd, 0, sizeof(rq->cmd));
 		rq->cmd[0] = GPCMD_READ_CD;
 		rq->cmd[1] = 1 << 2;
 		rq->cmd[2] = (lba >> 24) & 0xff;
diff --git a/drivers/md/dm-emc.c b/drivers/md/dm-emc.c
index 6b91b9a..3ea5ad4 100644
--- a/drivers/md/dm-emc.c
+++ b/drivers/md/dm-emc.c
@@ -110,8 +110,6 @@ static struct request *get_failover_req(struct emc_handler *h,
 	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
 	rq->sense_len = 0;
 
-	memset(&rq->cmd, 0, BLK_MAX_CDB);
-
 	rq->timeout = EMC_FAILOVER_TIMEOUT;
 	rq->cmd_type = REQ_TYPE_BLOCK_PC;
 	rq->cmd_flags |= REQ_FAILFAST | REQ_NOMERGE;
diff --git a/drivers/md/dm-mpath-hp-sw.c b/drivers/md/dm-mpath-hp-sw.c
index 204bf42..b63a0ab 100644
--- a/drivers/md/dm-mpath-hp-sw.c
+++ b/drivers/md/dm-mpath-hp-sw.c
@@ -137,7 +137,6 @@ static struct request *hp_sw_get_request(struct dm_path *path)
 	req->sense = h->sense;
 	memset(req->sense, 0, SCSI_SENSE_BUFFERSIZE);
 
-	memset(&req->cmd, 0, BLK_MAX_CDB);
 	req->cmd[0] = START_STOP;
 	req->cmd[4] = 1;
 	req->cmd_len = COMMAND_SIZE(req->cmd[0]);
diff --git a/drivers/md/dm-mpath-rdac.c b/drivers/md/dm-mpath-rdac.c
index e04eb5c..95e7773 100644
--- a/drivers/md/dm-mpath-rdac.c
+++ b/drivers/md/dm-mpath-rdac.c
@@ -284,7 +284,6 @@ static struct request *get_rdac_req(struct rdac_handler *h,
 		return NULL;
 	}
 
- 	memset(&rq->cmd, 0, BLK_MAX_CDB);
 	rq->sense = h->sense;
 	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
 	rq->sense_len = 0;
-- 
1.5.4.2

[PATCH 3/6] block: rename and export rq_init()

[FUJITA Tomonori]

This rename rq_init() blk_rq_init() and export it. Any path that hands
the request to the block layer needs to call it to initialize the
request.

This is a preparation for large command support, which needs to
initialize the request in a proper way (that is, just doing a memset()
will not work).

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

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 722140a..5e2cfe9 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -143,7 +143,7 @@ static void queue_flush(struct request_queue *q, unsigned which)
 		end_io = post_flush_end_io;
 	}
 
-	rq_init(q, rq);
+	blk_rq_init(q, rq);
 	rq->cmd_flags = REQ_HARDBARRIER;
 	rq->rq_disk = q->bar_rq.rq_disk;
 	rq->end_io = end_io;
@@ -165,7 +165,7 @@ static inline struct request *start_ordered(struct request_queue *q,
 	blkdev_dequeue_request(rq);
 	q->orig_bar_rq = rq;
 	rq = &q->bar_rq;
-	rq_init(q, rq);
+	blk_rq_init(q, rq);
 	if (bio_data_dir(q->orig_bar_rq->bio) == WRITE)
 		rq->cmd_flags |= REQ_RW;
 	if (q->ordered & QUEUE_ORDERED_FUA)
diff --git a/block/blk-core.c b/block/blk-core.c
index e7cdabc..a89adcc 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -107,7 +107,7 @@ struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev)
 }
 EXPORT_SYMBOL(blk_get_backing_dev_info);
 
-void rq_init(struct request_queue *q, struct request *rq)
+void blk_rq_init(struct request_queue *q, struct request *rq)
 {
 	memset(rq, 0, sizeof(*rq));
 
@@ -120,6 +120,7 @@ void rq_init(struct request_queue *q, struct request *rq)
 	rq->tag = -1;
 	rq->ref_count = 1;
 }
+EXPORT_SYMBOL(blk_rq_init);
 
 static void req_bio_endio(struct request *rq, struct bio *bio,
 			  unsigned int nbytes, int error)
@@ -601,7 +602,7 @@ blk_alloc_request(struct request_queue *q, int rw, int priv, gfp_t gfp_mask)
 	if (!rq)
 		return NULL;
 
-	rq_init(q, rq);
+	blk_rq_init(q, rq);
 
 	/*
 	 * first three bits are identical in rq->cmd_flags and bio->bi_rw,
diff --git a/block/blk.h b/block/blk.h
index ec9120f..59776ab 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -10,7 +10,6 @@
 extern struct kmem_cache *blk_requestq_cachep;
 extern struct kobj_type blk_queue_ktype;
 
-void rq_init(struct request_queue *q, struct request *rq);
 void init_request_from_bio(struct request *req, struct bio *bio);
 void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
 			struct bio *bio);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8ca481c..d17032c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -607,6 +607,7 @@ extern int blk_register_queue(struct gendisk *disk);
 extern void blk_unregister_queue(struct gendisk *disk);
 extern void register_disk(struct gendisk *dev);
 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);
-- 
1.5.4.2

[PATCH 4/6] block: use blk_rq_init() to initialize the request

[FUJITA Tomonori]

Any path needs to call it to initialize the request.

This is a preparation for large command support, which needs to
initialize the request in a proper way (that is, just doing a memset()
will not work).

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 60cc543..f75bda1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -537,6 +537,7 @@ static int nbd_ioctl(struct inode *inode, struct file *file,
 	switch (cmd) {
 	case NBD_DISCONNECT:
 	        printk(KERN_INFO "%s: NBD_DISCONNECT\n", lo->disk->disk_name);
+		blk_rq_init(NULL, &sreq);
 		sreq.cmd_type = REQ_TYPE_SPECIAL;
 		nbd_cmd(&sreq) = NBD_CMD_DISC;
 		/*
diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index df819f8..570f3b7 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -716,10 +716,8 @@ static int pd_special_command(struct pd_unit *disk,
 	struct request rq;
 	int err = 0;
 
-	memset(&rq, 0, sizeof(rq));
-	rq.errors = 0;
+	blk_rq_init(NULL, &rq);
 	rq.rq_disk = disk->gd;
-	rq.ref_count = 1;
 	rq.end_io_data = &wait;
 	rq.end_io = blk_end_sync_rq;
 	blk_insert_request(disk->gd->queue, &rq, 0, func);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 221f31e..1eaba6c 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1771,6 +1771,7 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
 	unsigned long flags;
 	int rtn;
 
+	blk_rq_init(NULL, &req);
 	scmd->request = &req;
 	memset(&scmd->eh_timeout, 0, sizeof(scmd->eh_timeout));
 
-- 
1.5.4.2

[PATCH 5/6] ide: use blk_rq_init() to initialize the request

[FUJITA Tomonori]

This converts ide to use blk_rq_init to initialize the request.

This is a preparation for large command support, which needs to
initialize the request in a proper way (that is, just doing a memset()
will not work).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-io.c       |    3 +--
 drivers/ide/ide-tape.c     |    2 +-
 drivers/ide/ide-taskfile.c |    3 +--
 drivers/ide/ide.c          |    4 ++--
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 31e5afa..7a3d605 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -1601,8 +1601,7 @@ irqreturn_t ide_intr (int irq, void *dev_id)
 
 void ide_init_drive_cmd (struct request *rq)
 {
-	memset(rq, 0, sizeof(*rq));
-	rq->ref_count = 1;
+	blk_rq_init(NULL, rq);
 }
 
 EXPORT_SYMBOL(ide_init_drive_cmd);
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index f43fd07..f1227e5 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -899,7 +899,7 @@ static void idetape_create_request_sense_cmd(struct ide_atapi_pc *pc)
 
 static void idetape_init_rq(struct request *rq, u8 cmd)
 {
-	memset(rq, 0, sizeof(*rq));
+	blk_rq_init(NULL, rq);
 	rq->cmd_type = REQ_TYPE_SPECIAL;
 	rq->cmd[0] = cmd;
 }
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index 155cc90..997b99c 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -532,8 +532,7 @@ int ide_raw_taskfile(ide_drive_t *drive, ide_task_t *task, u8 *buf, u16 nsect)
 {
 	struct request rq;
 
-	memset(&rq, 0, sizeof(rq));
-	rq.ref_count = 1;
+	blk_rq_init(NULL, &rq);
 	rq.cmd_type = REQ_TYPE_ATA_TASKFILE;
 	rq.buffer = buf;
 
diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index 917c72d..dfe3ba5 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -689,7 +689,7 @@ static int generic_ide_suspend(struct device *dev, pm_message_t mesg)
 	if (!(drive->dn % 2))
 		ide_acpi_get_timing(hwif);
 
-	memset(&rq, 0, sizeof(rq));
+	blk_rq_init(NULL, &rq);
 	memset(&rqpm, 0, sizeof(rqpm));
 	memset(&args, 0, sizeof(args));
 	rq.cmd_type = REQ_TYPE_PM_SUSPEND;
@@ -727,7 +727,7 @@ static int generic_ide_resume(struct device *dev)
 
 	ide_acpi_exec_tfs(drive);
 
-	memset(&rq, 0, sizeof(rq));
+	blk_rq_init(NULL, &rq);
 	memset(&rqpm, 0, sizeof(rqpm));
 	memset(&args, 0, sizeof(args));
 	rq.cmd_type = REQ_TYPE_PM_RESUME;
-- 
1.5.4.2

[PATCH 6/6] block: add large command support

[FUJITA Tomonori]

This patch changes rq->cmd from the static array to a pointer to
support large commands.

We rarely handle large commands. So for optimization, a struct request
still has a static array for a command. rq_init sets rq->cmd pointer
to the static array.

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

diff --git a/block/blk-core.c b/block/blk-core.c
index a89adcc..35ae1c0 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -117,6 +117,7 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
 	rq->sector = rq->hard_sector = (sector_t) -1;
 	INIT_HLIST_NODE(&rq->hash);
 	RB_CLEAR_NODE(&rq->rb_node);
+	rq->cmd = rq->__cmd;
 	rq->tag = -1;
 	rq->ref_count = 1;
 }
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d17032c..08df1ea 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -215,8 +215,9 @@ struct request {
 	/*
 	 * when request is used as a packet command carrier
 	 */
-	unsigned int cmd_len;
-	unsigned char cmd[BLK_MAX_CDB];
+	unsigned short cmd_len;
+	unsigned char __cmd[BLK_MAX_CDB];
+	unsigned char *cmd;
 
 	unsigned int data_len;
 	unsigned int extra_len;	/* length of alignment and padding */
-- 
1.5.4.2

[PATCH 1/6] block: no need to initialize rq->cmd in prepare_flush_fn hook

[FUJITA Tomonori]

The block layer initializes rq->cmd (queue_flush calls rq_init) so
prepare_flush_fn hooks don't need to do that.

The purpose of this patch is to remove sizeof(rq->cmd), as a
preparation for large command support, which changes rq->cmd from the
static array to a pointer. sizeof(rq->cmd) will not make sense.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
 drivers/block/ps3disk.c |    1 -
 drivers/scsi/sd.c       |    1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index 78e9ea7..d797e20 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -405,7 +405,6 @@ static void ps3disk_prepare_flush(struct request_queue *q, struct request *req)
 
 	dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
 
-	memset(req->cmd, 0, sizeof(req->cmd));
 	req->cmd_type = REQ_TYPE_FLUSH;
 }
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3cea17d..01cefbb 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -860,7 +860,6 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
 
 static void sd_prepare_flush(struct request_queue *q, struct request *rq)
 {
-	memset(rq->cmd, 0, sizeof(rq->cmd));
 	rq->cmd_type = REQ_TYPE_BLOCK_PC;
 	rq->timeout = SD_TIMEOUT;
 	rq->cmd[0] = SYNCHRONIZE_CACHE;
-- 
1.5.4.2

Re: [dm-devel] [PATCH 2/6] block: no need to initialize rq->cmd with blk_get_request

[James Bottomley]

On Sat, 2008-04-26 at 01:23 +0900, FUJITA Tomonori wrote:
> blk_get_request initializes rq->cmd (rq_init does) so the users don't
> need to do that.
> 
> The purpose of this patch is to remove sizeof(rq->cmd) and &rq->cmd,
> as a preparation for large command support, which changes rq->cmd from
> the static array to a pointer. sizeof(rq->cmd) will not make sense and
> &rq->cmd won't work.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Alasdair G Kergon <agk@redhat.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> ---
>  block/scsi_ioctl.c          |    3 ---
>  drivers/block/pktcdvd.c     |    2 --
>  drivers/cdrom/cdrom.c       |    1 -

These:

>  drivers/md/dm-emc.c         |    2 --
>  drivers/md/dm-mpath-hp-sw.c |    1 -
>  drivers/md/dm-mpath-rdac.c  |    1 -

Conflict with this set of patches:

http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-pending-2.6.git;a=summary

Which move the dm pieces to being request based.  The patch series
hasn't had an ACK yet from Alasdair, but it would be nice if you could
look it over and see if it needs the memsets pulling out.

Thanks,

James

Re: [dm-devel] [PATCH 2/6] block: no need to initialize rq->cmd with blk_get_request

[FUJITA Tomonori]

On Fri, 25 Apr 2008 11:45:01 -0500
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Sat, 2008-04-26 at 01:23 +0900, FUJITA Tomonori wrote:
> > blk_get_request initializes rq->cmd (rq_init does) so the users don't
> > need to do that.
> > 
> > The purpose of this patch is to remove sizeof(rq->cmd) and &rq->cmd,
> > as a preparation for large command support, which changes rq->cmd from
> > the static array to a pointer. sizeof(rq->cmd) will not make sense and
> > &rq->cmd won't work.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Cc: Alasdair G Kergon <agk@redhat.com>
> > Cc: Jens Axboe <jens.axboe@oracle.com>
> > ---
> >  block/scsi_ioctl.c          |    3 ---
> >  drivers/block/pktcdvd.c     |    2 --
> >  drivers/cdrom/cdrom.c       |    1 -
> 
> These:
> 
> >  drivers/md/dm-emc.c         |    2 --
> >  drivers/md/dm-mpath-hp-sw.c |    1 -
> >  drivers/md/dm-mpath-rdac.c  |    1 -
> 
> Conflict with this set of patches:
> 
> http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-pending-2.6.git;a=summary
> 
> Which move the dm pieces to being request based.  The patch series
> hasn't had an ACK yet from Alasdair, but it would be nice if you could
> look it over and see if it needs the memsets pulling out.

Seems that it needs the memsets pulling out. For example,

[PATCH 2/7] scsi_dh: add lsi rdac device handler has:

+	memset(&rq->cmd, 0, BLK_MAX_CDB);


Do you want a patch against scsi-pending?

Re: [dm-devel] [PATCH 2/6] block: no need to initialize rq->cmd with blk_get_request

[James Bottomley]

On Sat, 2008-04-26 at 01:54 +0900, FUJITA Tomonori wrote:
> On Fri, 25 Apr 2008 11:45:01 -0500
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > On Sat, 2008-04-26 at 01:23 +0900, FUJITA Tomonori wrote:
> > > blk_get_request initializes rq->cmd (rq_init does) so the users don't
> > > need to do that.
> > > 
> > > The purpose of this patch is to remove sizeof(rq->cmd) and &rq->cmd,
> > > as a preparation for large command support, which changes rq->cmd from
> > > the static array to a pointer. sizeof(rq->cmd) will not make sense and
> > > &rq->cmd won't work.
> > > 
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > Cc: Alasdair G Kergon <agk@redhat.com>
> > > Cc: Jens Axboe <jens.axboe@oracle.com>
> > > ---
> > >  block/scsi_ioctl.c          |    3 ---
> > >  drivers/block/pktcdvd.c     |    2 --
> > >  drivers/cdrom/cdrom.c       |    1 -
> > 
> > These:
> > 
> > >  drivers/md/dm-emc.c         |    2 --
> > >  drivers/md/dm-mpath-hp-sw.c |    1 -
> > >  drivers/md/dm-mpath-rdac.c  |    1 -
> > 
> > Conflict with this set of patches:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-pending-2.6.git;a=summary
> > 
> > Which move the dm pieces to being request based.  The patch series
> > hasn't had an ACK yet from Alasdair, but it would be nice if you could
> > look it over and see if it needs the memsets pulling out.
> 
> Seems that it needs the memsets pulling out. For example,
> 
> [PATCH 2/7] scsi_dh: add lsi rdac device handler has:
> 
> +	memset(&rq->cmd, 0, BLK_MAX_CDB);
> 
> 
> Do you want a patch against scsi-pending?

Yes please ...

Thanks,

James

Re: [PATCH 2/6] block: no need to initialize rq->cmd with blk_get_request

[FUJITA Tomonori]

On Fri, 25 Apr 2008 11:59:44 -0500
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Sat, 2008-04-26 at 01:54 +0900, FUJITA Tomonori wrote:
> > On Fri, 25 Apr 2008 11:45:01 -0500
> > James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> > 
> > > On Sat, 2008-04-26 at 01:23 +0900, FUJITA Tomonori wrote:
> > > > blk_get_request initializes rq->cmd (rq_init does) so the users don't
> > > > need to do that.
> > > > 
> > > > The purpose of this patch is to remove sizeof(rq->cmd) and &rq->cmd,
> > > > as a preparation for large command support, which changes rq->cmd from
> > > > the static array to a pointer. sizeof(rq->cmd) will not make sense and
> > > > &rq->cmd won't work.
> > > > 
> > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > > Cc: Alasdair G Kergon <agk@redhat.com>
> > > > Cc: Jens Axboe <jens.axboe@oracle.com>
> > > > ---
> > > >  block/scsi_ioctl.c          |    3 ---
> > > >  drivers/block/pktcdvd.c     |    2 --
> > > >  drivers/cdrom/cdrom.c       |    1 -
> > > 
> > > These:
> > > 
> > > >  drivers/md/dm-emc.c         |    2 --
> > > >  drivers/md/dm-mpath-hp-sw.c |    1 -
> > > >  drivers/md/dm-mpath-rdac.c  |    1 -
> > > 
> > > Conflict with this set of patches:
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-pending-2.6.git;a=summary
> > > 
> > > Which move the dm pieces to being request based.  The patch series
> > > hasn't had an ACK yet from Alasdair, but it would be nice if you could
> > > look it over and see if it needs the memsets pulling out.
> > 
> > Seems that it needs the memsets pulling out. For example,
> > 
> > [PATCH 2/7] scsi_dh: add lsi rdac device handler has:
> > 
> > +	memset(&rq->cmd, 0, BLK_MAX_CDB);
> > 
> > 
> > Do you want a patch against scsi-pending?
> 
> Yes please ...

You can apply this for scsi-pending now.

scsi_dh will not be merge for 2.6.26, the original patch needs to be
merged. If scsi_dh will, then we can drop the original patch since we
can live without the changes to scsi_ioctl.c, pktcdvd.c, and cdrom.c
(sizeof(rq->cmd) doesn't make sense but it doesn't hurt us).

==
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] scsi_dh: no need to initialize rq->cmd with blk_get_request

blk_get_request initializes rq->cmd (rq_init does) so the users don't
need to do that.

The purpose of this patch is to remove &rq->cmd, as a preparation for
large command support, which changes rq->cmd from the static array to
a pointer. &rq->cmd won't work.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/device_handler/scsi_dh_emc.c   |    1 -
 drivers/scsi/device_handler/scsi_dh_hp_sw.c |    1 -
 drivers/scsi/device_handler/scsi_dh_rdac.c  |    1 -
 3 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index 71d20c8..df7992c 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -258,7 +258,6 @@ static struct request *get_req(struct scsi_device *sdev, int cmd)
 		return NULL;
 	}
 
-	memset(&rq->cmd, 0, BLK_MAX_CDB);
 	rq->cmd[0] = cmd;
 	rq->cmd_len = COMMAND_SIZE(rq->cmd[0]);
 
diff --git a/drivers/scsi/device_handler/scsi_dh_hp_sw.c b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
index d1b66c3..7abea59 100644
--- a/drivers/scsi/device_handler/scsi_dh_hp_sw.c
+++ b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
@@ -91,7 +91,6 @@ static int hp_sw_activate(struct scsi_device *sdev)
 	req->cmd_type = REQ_TYPE_BLOCK_PC;
 	req->cmd_flags |= REQ_FAILFAST;
 	req->cmd_len = COMMAND_SIZE(START_STOP);
-	memset(req->cmd, 0, MAX_COMMAND_SIZE);
 	req->cmd[0] = START_STOP;
 	req->cmd[4] = 1;	/* Start spin cycle */
 	req->timeout = HP_SW_TIMEOUT;
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index 1bf9cf8..3af8fa9 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -214,7 +214,6 @@ static struct request *get_rdac_req(struct scsi_device *sdev,
 		return NULL;
 	}
 
-	memset(&rq->cmd, 0, BLK_MAX_CDB);
 	rq->sense = h->sense;
 	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
 	rq->sense_len = 0;
-- 
1.5.4.2

Re: [PATCH 3/6] block: rename and export rq_init()

[Boaz Harrosh]

On Fri, Apr 25 2008 at 19:23 +0300, FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> This rename rq_init() blk_rq_init() and export it. Any path that hands
> the request to the block layer needs to call it to initialize the
> request.
> 
> This is a preparation for large command support, which needs to
> initialize the request in a proper way (that is, just doing a memset()
> will not work).
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: Jens Axboe <jens.axboe@oracle.com>

Please, if not to late, add:
Cc: Boaz Harrosh <bharrosh@panasas.com>

> ---
>  block/blk-barrier.c    |    4 ++--
>  block/blk-core.c       |    5 +++--
>  block/blk.h            |    1 -
>  include/linux/blkdev.h |    1 +
>  4 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/block/blk-barrier.c b/block/blk-barrier.c
> index 722140a..5e2cfe9 100644
> --- a/block/blk-barrier.c
> +++ b/block/blk-barrier.c
> @@ -143,7 +143,7 @@ static void queue_flush(struct request_queue *q, unsigned which)
>  		end_io = post_flush_end_io;
>  	}
>  
> -	rq_init(q, rq);
> +	blk_rq_init(q, rq);
>  	rq->cmd_flags = REQ_HARDBARRIER;
>  	rq->rq_disk = q->bar_rq.rq_disk;
>  	rq->end_io = end_io;
> @@ -165,7 +165,7 @@ static inline struct request *start_ordered(struct request_queue *q,
>  	blkdev_dequeue_request(rq);
>  	q->orig_bar_rq = rq;
>  	rq = &q->bar_rq;
> -	rq_init(q, rq);
> +	blk_rq_init(q, rq);
>  	if (bio_data_dir(q->orig_bar_rq->bio) == WRITE)
>  		rq->cmd_flags |= REQ_RW;
>  	if (q->ordered & QUEUE_ORDERED_FUA)
> diff --git a/block/blk-core.c b/block/blk-core.c
> index e7cdabc..a89adcc 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -107,7 +107,7 @@ struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev)
>  }
>  EXPORT_SYMBOL(blk_get_backing_dev_info);
>  
> -void rq_init(struct request_queue *q, struct request *rq)
> +void blk_rq_init(struct request_queue *q, struct request *rq)
>  {
>  	memset(rq, 0, sizeof(*rq));
>  
> @@ -120,6 +120,7 @@ void rq_init(struct request_queue *q, struct request *rq)
>  	rq->tag = -1;
>  	rq->ref_count = 1;
>  }
> +EXPORT_SYMBOL(blk_rq_init);
>  
>  static void req_bio_endio(struct request *rq, struct bio *bio,
>  			  unsigned int nbytes, int error)
> @@ -601,7 +602,7 @@ blk_alloc_request(struct request_queue *q, int rw, int priv, gfp_t gfp_mask)
>  	if (!rq)
>  		return NULL;
>  
> -	rq_init(q, rq);
> +	blk_rq_init(q, rq);
>  
>  	/*
>  	 * first three bits are identical in rq->cmd_flags and bio->bi_rw,
> diff --git a/block/blk.h b/block/blk.h
> index ec9120f..59776ab 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -10,7 +10,6 @@
>  extern struct kmem_cache *blk_requestq_cachep;
>  extern struct kobj_type blk_queue_ktype;
>  
> -void rq_init(struct request_queue *q, struct request *rq);
>  void init_request_from_bio(struct request *req, struct bio *bio);
>  void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
>  			struct bio *bio);
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 8ca481c..d17032c 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -607,6 +607,7 @@ extern int blk_register_queue(struct gendisk *disk);
>  extern void blk_unregister_queue(struct gendisk *disk);
>  extern void register_disk(struct gendisk *dev);
>  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);

Re: [PATCH 4/6] block: use blk_rq_init() to initialize the request

[Boaz Harrosh]

On Fri, Apr 25 2008 at 19:23 +0300, FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> Any path needs to call it to initialize the request.
> 
> This is a preparation for large command support, which needs to
> initialize the request in a proper way (that is, just doing a memset()
> will not work).
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: Jens Axboe <jens.axboe@oracle.com>

Please, if not to late, add:
Cc: Boaz Harrosh <bharrosh@panasas.com>

> ---
>  drivers/block/nbd.c       |    1 +
>  drivers/block/paride/pd.c |    4 +---
>  drivers/scsi/scsi_error.c |    1 +
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 60cc543..f75bda1 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -537,6 +537,7 @@ static int nbd_ioctl(struct inode *inode, struct file *file,
>  	switch (cmd) {
>  	case NBD_DISCONNECT:
>  	        printk(KERN_INFO "%s: NBD_DISCONNECT\n", lo->disk->disk_name);
> +		blk_rq_init(NULL, &sreq);
>  		sreq.cmd_type = REQ_TYPE_SPECIAL;
>  		nbd_cmd(&sreq) = NBD_CMD_DISC;
>  		/*
> diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
> index df819f8..570f3b7 100644
> --- a/drivers/block/paride/pd.c
> +++ b/drivers/block/paride/pd.c
> @@ -716,10 +716,8 @@ static int pd_special_command(struct pd_unit *disk,
>  	struct request rq;
>  	int err = 0;
>  
> -	memset(&rq, 0, sizeof(rq));
> -	rq.errors = 0;
> +	blk_rq_init(NULL, &rq);
>  	rq.rq_disk = disk->gd;
> -	rq.ref_count = 1;
>  	rq.end_io_data = &wait;
>  	rq.end_io = blk_end_sync_rq;
>  	blk_insert_request(disk->gd->queue, &rq, 0, func);
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 221f31e..1eaba6c 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1771,6 +1771,7 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
>  	unsigned long flags;
>  	int rtn;
>  
> +	blk_rq_init(NULL, &req);
>  	scmd->request = &req;
>  	memset(&scmd->eh_timeout, 0, sizeof(scmd->eh_timeout));
>  

Re: [PATCH 5/6] ide: use blk_rq_init() to initialize the request

[Boaz Harrosh]

On Fri, Apr 25 2008 at 19:23 +0300, FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> This converts ide to use blk_rq_init to initialize the request.
> 
> This is a preparation for large command support, which needs to
> initialize the request in a proper way (that is, just doing a memset()
> will not work).
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Please, if not to late, add:
Cc: Boaz Harrosh <bharrosh@panasas.com>

> ---
>  drivers/ide/ide-io.c       |    3 +--
>  drivers/ide/ide-tape.c     |    2 +-
>  drivers/ide/ide-taskfile.c |    3 +--
>  drivers/ide/ide.c          |    4 ++--
>  4 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
> index 31e5afa..7a3d605 100644
> --- a/drivers/ide/ide-io.c
> +++ b/drivers/ide/ide-io.c
> @@ -1601,8 +1601,7 @@ irqreturn_t ide_intr (int irq, void *dev_id)
>  
>  void ide_init_drive_cmd (struct request *rq)
>  {
> -	memset(rq, 0, sizeof(*rq));
> -	rq->ref_count = 1;
> +	blk_rq_init(NULL, rq);
>  }
>  
>  EXPORT_SYMBOL(ide_init_drive_cmd);
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index f43fd07..f1227e5 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -899,7 +899,7 @@ static void idetape_create_request_sense_cmd(struct ide_atapi_pc *pc)
>  
>  static void idetape_init_rq(struct request *rq, u8 cmd)
>  {
> -	memset(rq, 0, sizeof(*rq));
> +	blk_rq_init(NULL, rq);
>  	rq->cmd_type = REQ_TYPE_SPECIAL;
>  	rq->cmd[0] = cmd;
>  }
> diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> index 155cc90..997b99c 100644
> --- a/drivers/ide/ide-taskfile.c
> +++ b/drivers/ide/ide-taskfile.c
> @@ -532,8 +532,7 @@ int ide_raw_taskfile(ide_drive_t *drive, ide_task_t *task, u8 *buf, u16 nsect)
>  {
>  	struct request rq;
>  
> -	memset(&rq, 0, sizeof(rq));
> -	rq.ref_count = 1;
> +	blk_rq_init(NULL, &rq);
>  	rq.cmd_type = REQ_TYPE_ATA_TASKFILE;
>  	rq.buffer = buf;
>  
> diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
> index 917c72d..dfe3ba5 100644
> --- a/drivers/ide/ide.c
> +++ b/drivers/ide/ide.c
> @@ -689,7 +689,7 @@ static int generic_ide_suspend(struct device *dev, pm_message_t mesg)
>  	if (!(drive->dn % 2))
>  		ide_acpi_get_timing(hwif);
>  
> -	memset(&rq, 0, sizeof(rq));
> +	blk_rq_init(NULL, &rq);
>  	memset(&rqpm, 0, sizeof(rqpm));
>  	memset(&args, 0, sizeof(args));
>  	rq.cmd_type = REQ_TYPE_PM_SUSPEND;
> @@ -727,7 +727,7 @@ static int generic_ide_resume(struct device *dev)
>  
>  	ide_acpi_exec_tfs(drive);
>  
> -	memset(&rq, 0, sizeof(rq));
> +	blk_rq_init(NULL, &rq);
>  	memset(&rqpm, 0, sizeof(rqpm));
>  	memset(&args, 0, sizeof(args));
>  	rq.cmd_type = REQ_TYPE_PM_RESUME;

Re: [PATCH 6/6] block: add large command support

[Boaz Harrosh]

On Fri, Apr 25 2008 at 19:23 +0300, FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> This patch changes rq->cmd from the static array to a pointer to
> support large commands.
> 
> We rarely handle large commands. So for optimization, a struct request
> still has a static array for a command. rq_init sets rq->cmd pointer
> to the static array.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: Jens Axboe <jens.axboe@oracle.com>

Please, if not to late, add:
Cc: Boaz Harrosh <bharrosh@panasas.com>

> ---
>  block/blk-core.c       |    1 +
>  include/linux/blkdev.h |    5 +++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index a89adcc..35ae1c0 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -117,6 +117,7 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
>  	rq->sector = rq->hard_sector = (sector_t) -1;
>  	INIT_HLIST_NODE(&rq->hash);
>  	RB_CLEAR_NODE(&rq->rb_node);
> +	rq->cmd = rq->__cmd;
>  	rq->tag = -1;
>  	rq->ref_count = 1;
>  }
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index d17032c..08df1ea 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -215,8 +215,9 @@ struct request {
>  	/*
>  	 * when request is used as a packet command carrier
>  	 */
> -	unsigned int cmd_len;
> -	unsigned char cmd[BLK_MAX_CDB];
> +	unsigned short cmd_len;
> +	unsigned char __cmd[BLK_MAX_CDB];
> +	unsigned char *cmd;
>  
>  	unsigned int data_len;
>  	unsigned int extra_len;	/* length of alignment and padding */

Re: [PATCH 0/6] add large command support to the block layer

[Jens Axboe]

On Sat, Apr 26 2008, FUJITA Tomonori wrote:
> This is an updated patchset for large command support to the block
> layer:
> 
> http://marc.info/?l=linux-scsi&m=120817127118449&w=2
> 
> We rarely handle large commands. So for optimization, a struct request
> still has a static array for a command. rq_init sets rq->cmd pointer
> to the static array. In short, rq_init() does
> 
> rq->cmd = rq->__cmd;
> 
> So we can access to rq->cmd and rq->cmd_len as before.
> 
> This change requires everyone to initialize the request in a proper
> way (that is, just doing a memset() will not work). Now we have
> rq_init() that works for any path so this patchset can cleanly convert
> users of requests on the stack or kmalloced requests to use it (the
> previous patchset does it in a hacky way):
> 
> http://marc.info/?l=linux-scsi&m=120911792725876&w=2
> 
> This patchset is against Jens' for-linus branch.
> 
> #1-#4 patches can be applied via Jens' tree now. #5 patch is for
> IDE. It cleanly can be applied to both Bart's latest quilt tree and
> Jens' tree though Bart's quilt tree has some pending IDE patches. #4
> patch depends on #4. #6 patch depends on #1-#5.
> 
> I guess that the easiest way to apply this patchset would be:
> 
> 1. Pushing Bart's quilt tree to mainline.
> 2. Rebasing Jens' tree to mainline.
> 3. Pushing this patchset via Jens' tree.
> 
> Jens and Bart, let me know if I can do something to make the process
> easier.
> 
> Bart, I will try to push the patchset to remove the requests on the
> stack for 2.6.27:
> 
> http://marc.info/?l=linux-ide&m=120882410712466&w=2

I've applied all patches to the for-linus branch, it should go up
soonish. If anyone has problems with this, please holler SOON.

-- 
Jens Axboe

Re: [PATCH 0/6] add large command support to the block layer

[Bartlomiej Zolnierkiewicz]

On Tue, Apr 29, 2008 at 9:54 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
>
> On Sat, Apr 26 2008, FUJITA Tomonori wrote:
>  > This is an updated patchset for large command support to the block
>  > layer:
>  >
>  > http://marc.info/?l=linux-scsi&m=120817127118449&w=2
>  >
>  > We rarely handle large commands. So for optimization, a struct request
>  > still has a static array for a command. rq_init sets rq->cmd pointer
>  > to the static array. In short, rq_init() does
>  >
>  > rq->cmd = rq->__cmd;
>  >
>  > So we can access to rq->cmd and rq->cmd_len as before.
>  >
>  > This change requires everyone to initialize the request in a proper
>  > way (that is, just doing a memset() will not work). Now we have
>  > rq_init() that works for any path so this patchset can cleanly convert
>  > users of requests on the stack or kmalloced requests to use it (the
>  > previous patchset does it in a hacky way):
>  >
>  > http://marc.info/?l=linux-scsi&m=120911792725876&w=2
>  >
>  > This patchset is against Jens' for-linus branch.
>  >
>  > #1-#4 patches can be applied via Jens' tree now. #5 patch is for
>  > IDE. It cleanly can be applied to both Bart's latest quilt tree and
>  > Jens' tree though Bart's quilt tree has some pending IDE patches. #4
>  > patch depends on #4. #6 patch depends on #1-#5.
>  >
>  > I guess that the easiest way to apply this patchset would be:
>  >
>  > 1. Pushing Bart's quilt tree to mainline.
>  > 2. Rebasing Jens' tree to mainline.
>  > 3. Pushing this patchset via Jens' tree.
>  >
>  > Jens and Bart, let me know if I can do something to make the process
>  > easier.
>  >
>  > Bart, I will try to push the patchset to remove the requests on the
>  > stack for 2.6.27:
>  >
>  > http://marc.info/?l=linux-ide&m=120882410712466&w=2
>
>  I've applied all patches to the for-linus branch, it should go up
>  soonish. If anyone has problems with this, please holler SOON.

Fine with me (patches look good and survived quick testing).

My only concern is that the final series from Tomo lacked

	"block: replace sizeof(rq->cmd) with BLK_MAX_CDB"

and it is also not in for-linus branch (it has to be merged
before "block: add large command support" patch or ide-cd
will break).

Thanks,
Bart

Re: [PATCH 0/6] add large command support to the block layer

[FUJITA Tomonori]

On Tue, 29 Apr 2008 13:55:13 +0200
"Bartlomiej Zolnierkiewicz" <bzolnier@gmail.com> wrote:

> On Tue, Apr 29, 2008 at 9:54 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> >
> > On Sat, Apr 26 2008, FUJITA Tomonori wrote:
> >  > This is an updated patchset for large command support to the block
> >  > layer:
> >  >
> >  > http://marc.info/?l=linux-scsi&m=120817127118449&w=2
> >  >
> >  > We rarely handle large commands. So for optimization, a struct request
> >  > still has a static array for a command. rq_init sets rq->cmd pointer
> >  > to the static array. In short, rq_init() does
> >  >
> >  > rq->cmd = rq->__cmd;
> >  >
> >  > So we can access to rq->cmd and rq->cmd_len as before.
> >  >
> >  > This change requires everyone to initialize the request in a proper
> >  > way (that is, just doing a memset() will not work). Now we have
> >  > rq_init() that works for any path so this patchset can cleanly convert
> >  > users of requests on the stack or kmalloced requests to use it (the
> >  > previous patchset does it in a hacky way):
> >  >
> >  > http://marc.info/?l=linux-scsi&m=120911792725876&w=2
> >  >
> >  > This patchset is against Jens' for-linus branch.
> >  >
> >  > #1-#4 patches can be applied via Jens' tree now. #5 patch is for
> >  > IDE. It cleanly can be applied to both Bart's latest quilt tree and
> >  > Jens' tree though Bart's quilt tree has some pending IDE patches. #4
> >  > patch depends on #4. #6 patch depends on #1-#5.
> >  >
> >  > I guess that the easiest way to apply this patchset would be:
> >  >
> >  > 1. Pushing Bart's quilt tree to mainline.
> >  > 2. Rebasing Jens' tree to mainline.
> >  > 3. Pushing this patchset via Jens' tree.
> >  >
> >  > Jens and Bart, let me know if I can do something to make the process
> >  > easier.
> >  >
> >  > Bart, I will try to push the patchset to remove the requests on the
> >  > stack for 2.6.27:
> >  >
> >  > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> >
> >  I've applied all patches to the for-linus branch, it should go up
> >  soonish. If anyone has problems with this, please holler SOON.
> 
> Fine with me (patches look good and survived quick testing).

Thanks,


> My only concern is that the final series from Tomo lacked
> 
> 	"block: replace sizeof(rq->cmd) with BLK_MAX_CDB"
> 
> and it is also not in for-linus branch (it has to be merged
> before "block: add large command support" patch or ide-cd
> will break).

Sorry, somehow I forgot to put it in the patchset. Yeah, probabaly
initializing only 4 (or 8) bytes in rq->cmd would not work for ide-cd.

Jens, please put this to the for-linus branch.

==
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] block: replace sizeof(rq->cmd) with BLK_MAX_CDB

This is a preparation for changing rq->cmd from the static array to a
pointer.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Boaz Harrosh <bharrosh@panasas.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
---
 block/blk-core.c             |    2 +-
 drivers/ide/ide-cd.c         |    4 ++--
 drivers/ide/ide-cd_verbose.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 35ae1c0..abe5b9f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -174,7 +174,7 @@ void blk_dump_rq_flags(struct request *rq, char *msg)
 
 	if (blk_pc_request(rq)) {
 		printk(KERN_INFO "  cdb: ");
-		for (bit = 0; bit < sizeof(rq->cmd); bit++)
+		for (bit = 0; bit < BLK_MAX_CDB; bit++)
 			printk("%02x ", rq->cmd[bit]);
 		printk("\n");
 	}
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index fe9df38..68e7f19 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -782,7 +782,7 @@ static ide_startstop_t cdrom_start_seek_continuation(ide_drive_t *drive)
 
 	sector_div(frame, queue_hardsect_size(drive->queue) >> SECTOR_BITS);
 
-	memset(rq->cmd, 0, sizeof(rq->cmd));
+	memset(rq->cmd, 0, BLK_MAX_CDB);
 	rq->cmd[0] = GPCMD_SEEK;
 	put_unaligned(cpu_to_be32(frame), (unsigned int *) &rq->cmd[2]);
 
@@ -1694,7 +1694,7 @@ static int ide_cdrom_prep_fs(struct request_queue *q, struct request *rq)
 	long block = (long)rq->hard_sector / (hard_sect >> 9);
 	unsigned long blocks = rq->hard_nr_sectors / (hard_sect >> 9);
 
-	memset(rq->cmd, 0, sizeof(rq->cmd));
+	memset(rq->cmd, 0, BLK_MAX_CDB);
 
 	if (rq_data_dir(rq) == READ)
 		rq->cmd[0] = GPCMD_READ_10;
diff --git a/drivers/ide/ide-cd_verbose.c b/drivers/ide/ide-cd_verbose.c
index 6ed7ca0..6490a2d 100644
--- a/drivers/ide/ide-cd_verbose.c
+++ b/drivers/ide/ide-cd_verbose.c
@@ -326,7 +326,7 @@ void ide_cd_log_error(const char *name, struct request *failed_command,
 
 		printk(KERN_ERR "  The failed \"%s\" packet command "
 				"was: \n  \"", s);
-		for (i = 0; i < sizeof(failed_command->cmd); i++)
+		for (i = 0; i < BLK_MAX_CDB; i++)
 			printk(KERN_CONT "%02x ", failed_command->cmd[i]);
 		printk(KERN_CONT "\"\n");
 	}
-- 
1.5.4.2

Re: [PATCH 0/6] add large command support to the block layer

[Jens Axboe]

On Tue, Apr 29 2008, FUJITA Tomonori wrote:
> On Tue, 29 Apr 2008 13:55:13 +0200
> "Bartlomiej Zolnierkiewicz" <bzolnier@gmail.com> wrote:
> 
> > On Tue, Apr 29, 2008 at 9:54 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> > >
> > > On Sat, Apr 26 2008, FUJITA Tomonori wrote:
> > >  > This is an updated patchset for large command support to the block
> > >  > layer:
> > >  >
> > >  > http://marc.info/?l=linux-scsi&m=120817127118449&w=2
> > >  >
> > >  > We rarely handle large commands. So for optimization, a struct request
> > >  > still has a static array for a command. rq_init sets rq->cmd pointer
> > >  > to the static array. In short, rq_init() does
> > >  >
> > >  > rq->cmd = rq->__cmd;
> > >  >
> > >  > So we can access to rq->cmd and rq->cmd_len as before.
> > >  >
> > >  > This change requires everyone to initialize the request in a proper
> > >  > way (that is, just doing a memset() will not work). Now we have
> > >  > rq_init() that works for any path so this patchset can cleanly convert
> > >  > users of requests on the stack or kmalloced requests to use it (the
> > >  > previous patchset does it in a hacky way):
> > >  >
> > >  > http://marc.info/?l=linux-scsi&m=120911792725876&w=2
> > >  >
> > >  > This patchset is against Jens' for-linus branch.
> > >  >
> > >  > #1-#4 patches can be applied via Jens' tree now. #5 patch is for
> > >  > IDE. It cleanly can be applied to both Bart's latest quilt tree and
> > >  > Jens' tree though Bart's quilt tree has some pending IDE patches. #4
> > >  > patch depends on #4. #6 patch depends on #1-#5.
> > >  >
> > >  > I guess that the easiest way to apply this patchset would be:
> > >  >
> > >  > 1. Pushing Bart's quilt tree to mainline.
> > >  > 2. Rebasing Jens' tree to mainline.
> > >  > 3. Pushing this patchset via Jens' tree.
> > >  >
> > >  > Jens and Bart, let me know if I can do something to make the process
> > >  > easier.
> > >  >
> > >  > Bart, I will try to push the patchset to remove the requests on the
> > >  > stack for 2.6.27:
> > >  >
> > >  > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > >
> > >  I've applied all patches to the for-linus branch, it should go up
> > >  soonish. If anyone has problems with this, please holler SOON.
> > 
> > Fine with me (patches look good and survived quick testing).
> 
> Thanks,
> 
> 
> > My only concern is that the final series from Tomo lacked
> > 
> > 	"block: replace sizeof(rq->cmd) with BLK_MAX_CDB"
> > 
> > and it is also not in for-linus branch (it has to be merged
> > before "block: add large command support" patch or ide-cd
> > will break).
> 
> Sorry, somehow I forgot to put it in the patchset. Yeah, probabaly
> initializing only 4 (or 8) bytes in rq->cmd would not work for ide-cd.
> 
> Jens, please put this to the for-linus branch.

Done, please inspect the result (and ordering), thanks.

-- 
Jens Axboe

Re: [PATCH 0/6] add large command support to the block layer

[FUJITA Tomonori]

On Tue, 29 Apr 2008 14:37:12 +0200
Jens Axboe <jens.axboe@oracle.com> wrote:

> On Tue, Apr 29 2008, FUJITA Tomonori wrote:
> > On Tue, 29 Apr 2008 13:55:13 +0200
> > "Bartlomiej Zolnierkiewicz" <bzolnier@gmail.com> wrote:
> > 
> > > On Tue, Apr 29, 2008 at 9:54 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> > > >
> > > > On Sat, Apr 26 2008, FUJITA Tomonori wrote:
> > > >  > This is an updated patchset for large command support to the block
> > > >  > layer:
> > > >  >
> > > >  > http://marc.info/?l=linux-scsi&m=120817127118449&w=2
> > > >  >
> > > >  > We rarely handle large commands. So for optimization, a struct request
> > > >  > still has a static array for a command. rq_init sets rq->cmd pointer
> > > >  > to the static array. In short, rq_init() does
> > > >  >
> > > >  > rq->cmd = rq->__cmd;
> > > >  >
> > > >  > So we can access to rq->cmd and rq->cmd_len as before.
> > > >  >
> > > >  > This change requires everyone to initialize the request in a proper
> > > >  > way (that is, just doing a memset() will not work). Now we have
> > > >  > rq_init() that works for any path so this patchset can cleanly convert
> > > >  > users of requests on the stack or kmalloced requests to use it (the
> > > >  > previous patchset does it in a hacky way):
> > > >  >
> > > >  > http://marc.info/?l=linux-scsi&m=120911792725876&w=2
> > > >  >
> > > >  > This patchset is against Jens' for-linus branch.
> > > >  >
> > > >  > #1-#4 patches can be applied via Jens' tree now. #5 patch is for
> > > >  > IDE. It cleanly can be applied to both Bart's latest quilt tree and
> > > >  > Jens' tree though Bart's quilt tree has some pending IDE patches. #4
> > > >  > patch depends on #4. #6 patch depends on #1-#5.
> > > >  >
> > > >  > I guess that the easiest way to apply this patchset would be:
> > > >  >
> > > >  > 1. Pushing Bart's quilt tree to mainline.
> > > >  > 2. Rebasing Jens' tree to mainline.
> > > >  > 3. Pushing this patchset via Jens' tree.
> > > >  >
> > > >  > Jens and Bart, let me know if I can do something to make the process
> > > >  > easier.
> > > >  >
> > > >  > Bart, I will try to push the patchset to remove the requests on the
> > > >  > stack for 2.6.27:
> > > >  >
> > > >  > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > > >
> > > >  I've applied all patches to the for-linus branch, it should go up
> > > >  soonish. If anyone has problems with this, please holler SOON.
> > > 
> > > Fine with me (patches look good and survived quick testing).
> > 
> > Thanks,
> > 
> > 
> > > My only concern is that the final series from Tomo lacked
> > > 
> > > 	"block: replace sizeof(rq->cmd) with BLK_MAX_CDB"
> > > 
> > > and it is also not in for-linus branch (it has to be merged
> > > before "block: add large command support" patch or ide-cd
> > > will break).
> > 
> > Sorry, somehow I forgot to put it in the patchset. Yeah, probabaly
> > initializing only 4 (or 8) bytes in rq->cmd would not work for ide-cd.
> > 
> > Jens, please put this to the for-linus branch.
> 
> Done, please inspect the result (and ordering), thanks.

Thanks, it should work.

BTW, I think that the commit 09225d2 in for-linus branch (block: make
queue flags non-atomic) needs to export blk_run_queue.

Re: [PATCH 0/6] add large command support to the block layer

[Jens Axboe]

On Tue, Apr 29 2008, FUJITA Tomonori wrote:
> On Tue, 29 Apr 2008 14:37:12 +0200
> Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > On Tue, Apr 29 2008, FUJITA Tomonori wrote:
> > > On Tue, 29 Apr 2008 13:55:13 +0200
> > > "Bartlomiej Zolnierkiewicz" <bzolnier@gmail.com> wrote:
> > > 
> > > > On Tue, Apr 29, 2008 at 9:54 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> > > > >
> > > > > On Sat, Apr 26 2008, FUJITA Tomonori wrote:
> > > > >  > This is an updated patchset for large command support to the block
> > > > >  > layer:
> > > > >  >
> > > > >  > http://marc.info/?l=linux-scsi&m=120817127118449&w=2
> > > > >  >
> > > > >  > We rarely handle large commands. So for optimization, a struct request
> > > > >  > still has a static array for a command. rq_init sets rq->cmd pointer
> > > > >  > to the static array. In short, rq_init() does
> > > > >  >
> > > > >  > rq->cmd = rq->__cmd;
> > > > >  >
> > > > >  > So we can access to rq->cmd and rq->cmd_len as before.
> > > > >  >
> > > > >  > This change requires everyone to initialize the request in a proper
> > > > >  > way (that is, just doing a memset() will not work). Now we have
> > > > >  > rq_init() that works for any path so this patchset can cleanly convert
> > > > >  > users of requests on the stack or kmalloced requests to use it (the
> > > > >  > previous patchset does it in a hacky way):
> > > > >  >
> > > > >  > http://marc.info/?l=linux-scsi&m=120911792725876&w=2
> > > > >  >
> > > > >  > This patchset is against Jens' for-linus branch.
> > > > >  >
> > > > >  > #1-#4 patches can be applied via Jens' tree now. #5 patch is for
> > > > >  > IDE. It cleanly can be applied to both Bart's latest quilt tree and
> > > > >  > Jens' tree though Bart's quilt tree has some pending IDE patches. #4
> > > > >  > patch depends on #4. #6 patch depends on #1-#5.
> > > > >  >
> > > > >  > I guess that the easiest way to apply this patchset would be:
> > > > >  >
> > > > >  > 1. Pushing Bart's quilt tree to mainline.
> > > > >  > 2. Rebasing Jens' tree to mainline.
> > > > >  > 3. Pushing this patchset via Jens' tree.
> > > > >  >
> > > > >  > Jens and Bart, let me know if I can do something to make the process
> > > > >  > easier.
> > > > >  >
> > > > >  > Bart, I will try to push the patchset to remove the requests on the
> > > > >  > stack for 2.6.27:
> > > > >  >
> > > > >  > http://marc.info/?l=linux-ide&m=120882410712466&w=2
> > > > >
> > > > >  I've applied all patches to the for-linus branch, it should go up
> > > > >  soonish. If anyone has problems with this, please holler SOON.
> > > > 
> > > > Fine with me (patches look good and survived quick testing).
> > > 
> > > Thanks,
> > > 
> > > 
> > > > My only concern is that the final series from Tomo lacked
> > > > 
> > > > 	"block: replace sizeof(rq->cmd) with BLK_MAX_CDB"
> > > > 
> > > > and it is also not in for-linus branch (it has to be merged
> > > > before "block: add large command support" patch or ide-cd
> > > > will break).
> > > 
> > > Sorry, somehow I forgot to put it in the patchset. Yeah, probabaly
> > > initializing only 4 (or 8) bytes in rq->cmd would not work for ide-cd.
> > > 
> > > Jens, please put this to the for-linus branch.
> > 
> > Done, please inspect the result (and ordering), thanks.
> 
> Thanks, it should work.
> 
> BTW, I think that the commit 09225d2 in for-linus branch (block: make
> queue flags non-atomic) needs to export blk_run_queue.

Good spotting, it does. Fixed up.

-- 
Jens Axboe