[Iscsitarget-devel] [DRAFT] VAAI patch 1: implement WRITE_SAME

Yucong Sun (叶雨飞)

unread,

Mar 24, 2012, 3:43:03 AM3/24/12

to iscsitarget-devel

Hi, here's my first take on implementing WRITE_SAME_16 ,

which always has data size 1 (if I understand spec correctly),

below is the untested patch to verify that I'm in the correct direction,

Thanks.

sunyc@throne:~/iscsitarget/trunk$ cat write_same_16.patch
Index: kernel/target_disk.c
===================================================================
--- kernel/target_disk.c (revision 470)
+++ kernel/target_disk.c (working copy)
@@ -548,6 +548,7 @@
case WRITE_10:
case WRITE_16:
case WRITE_VERIFY:
+ case WRITE_SAME_16:
send_scsi_rsp(cmnd, build_write_response);
break;
case SYNCHRONIZE_CACHE:
Index: kernel/volume.c
===================================================================
--- kernel/volume.c (revision 470)
+++ kernel/volume.c (working copy)
@@ -445,6 +445,7 @@
case WRITE_12:
case WRITE_16:
case WRITE_VERIFY:
+ case WRITE_SAME_16:
case SYNCHRONIZE_CACHE:
if (write_excl || excl_access)
err = -EBUSY;
Index: kernel/iscsi.c
===================================================================
--- kernel/iscsi.c (revision 470)
+++ kernel/iscsi.c (working copy)
@@ -777,6 +777,7 @@
break;
case READ_16:
case WRITE_16:
+ case WRITE_SAME_16:
*off = (u64)cmd[2] << 56 | (u64)cmd[3] << 48 |
(u64)cmd[4] << 40 | (u64)cmd[5] << 32 |
(u64)cmd[6] << 24 | (u64)cmd[7] << 16 |
@@ -1039,6 +1040,7 @@
case WRITE_10:
case WRITE_16:
case WRITE_VERIFY:
+ case WRITE_SAME_16:
{
struct iscsi_sess_param *param = &conn->session->param;
loff_t offset;
@@ -1067,8 +1069,6 @@
eprintk("Verification is ignored %x\n", cmnd_itt(req));

set_offset_and_length(req->lun, req_hdr->scb, &offset, &length);
- if (cmnd_write_size(req) != length)
- eprintk("%x %u %u\n", cmnd_itt(req), cmnd_write_size(req), length);

req->tio = tio_alloc(get_pgcnt(length, offset));
tio_set(req->tio, length, offset);
@@ -1077,6 +1077,17 @@
if (cmnd_recv_pdu(conn, req->tio, 0, req->pdu.datasize) < 0)
assert(0);
}
+
+ /* WRITE_SAME_16 always have 1 data size, we pad the first page
+ to the rest of the pages. */
+ if (req_hdr->scb[0] == WRITE_SAME_16) {
+ tio_pad(req->tio, 0);
+ } else {
+ if (cmnd_write_size(req) != length) {
+ eprintk("%x %u %u\n",
+ cmnd_itt(req), cmnd_write_size(req), length);
+ }
+ }
break;
}
error:
Index: kernel/iscsi.h
===================================================================
--- kernel/iscsi.h (revision 470)
+++ kernel/iscsi.h (working copy)
@@ -414,6 +414,7 @@
extern void tio_get(struct tio *);
extern void tio_put(struct tio *);
extern void tio_set(struct tio *, u32, loff_t);
+extern void tio_pad(struct tio *, int);
extern int tio_read(struct iet_volume *, struct tio *);
extern int tio_write(struct iet_volume *, struct tio *);
extern int tio_sync(struct iet_volume *, struct tio *);
Index: kernel/tio.c
===================================================================
--- kernel/tio.c (revision 470)
+++ kernel/tio.c (working copy)
@@ -124,6 +124,21 @@
tio->size = size;
}

+void tio_pad(struct tio *tio, int idx)
+{
+ int i;
+ u8 *src, *dst;
+
+ assert(idx <= tio->pg_cnt - 1);
+ assert(tio->pvec[idx]);
+
+ src = page_address(tio->pvec[idx]);
+ for (i = idx + 1; i < tio->pg_cnt; i++) {
+ dst = page_address(tio->pvec[i]);
+ memcpy(dst, src, PAGE_SIZE);
+ }
+}
+
int tio_read(struct iet_volume *lu, struct tio *tio)
{
struct iotype *iot = lu->iotype;

Pasi Kärkkäinen

unread,

Mar 24, 2012, 4:08:57 AM3/24/12

to Yucong Sun (?????????), iscsitarget-devel

On Sat, Mar 24, 2012 at 12:43:03AM -0700, Yucong Sun (?????????) wrote:
> Hi, here's my first take on implementing WRITE_SAME_16 ,Â
> which always has data size 1 (if I understand spec correctly),Â
> below is the untested patch to verify that I'm in the correct direction,Â
> Thanks.
>

Hello,

Thanks a lot for the VAAI work!

> sunyc@throne:~/iscsitarget/trunk$ cat write_same_16.patch
> Index: kernel/target_disk.c
> ===================================================================

> --- kernel/target_disk.c Â Â Â Â (revision 470)
> +++ kernel/target_disk.c Â Â Â Â (working copy)
> @@ -548,6 +548,7 @@
> Â Â Â Â case WRITE_10:
> Â Â Â Â case WRITE_16:
> Â Â Â Â case WRITE_VERIFY:
> + Â Â Â case WRITE_SAME_16:
> Â Â Â Â Â Â Â Â send_scsi_rsp(cmnd, build_write_response);
> Â Â Â Â Â Â Â Â break;
> Â Â Â Â case SYNCHRONIZE_CACHE:

.. but this patch seems totally corrupted, at least for me..

-- Pasi

> Index: kernel/volume.c
> ===================================================================
> --- kernel/volume.c Â Â (revision 470)
> +++ kernel/volume.c Â Â (working copy)
> @@ -445,6 +445,7 @@
> Â Â Â Â case WRITE_12:
> Â Â Â Â case WRITE_16:
> Â Â Â Â case WRITE_VERIFY:
> + Â Â Â case WRITE_SAME_16:
> Â Â Â Â case SYNCHRONIZE_CACHE:
> Â Â Â Â Â Â Â Â if (write_excl || excl_access)
> Â Â Â Â Â Â Â Â Â Â Â Â err = -EBUSY;
> Index: kernel/iscsi.c
> ===================================================================
> --- kernel/iscsi.c Â Â Â (revision 470)
> +++ kernel/iscsi.c Â Â Â (working copy)
> @@ -777,6 +777,7 @@
> Â Â Â Â Â Â Â Â break;
> Â Â Â Â case READ_16:
> Â Â Â Â case WRITE_16:
> + Â Â Â case WRITE_SAME_16:
> Â Â Â Â Â Â Â Â *off = (u64)cmd[2] << 56 | (u64)cmd[3] << 48 |
> Â Â Â Â Â Â Â Â Â Â Â Â (u64)cmd[4] << 40 | (u64)cmd[5] << 32 |
> Â Â Â Â Â Â Â Â Â Â Â Â (u64)cmd[6] << 24 | (u64)cmd[7] << 16 |
> @@ -1039,6 +1040,7 @@
> Â Â Â Â case WRITE_10:
> Â Â Â Â case WRITE_16:
> Â Â Â Â case WRITE_VERIFY:
> + Â Â Â case WRITE_SAME_16:
> Â Â Â Â {
> Â Â Â Â Â Â Â Â struct iscsi_sess_param *param =
> &conn->session->param;
> Â Â Â Â Â Â Â Â loff_t offset;
> @@ -1067,8 +1069,6 @@
> Â Â Â Â Â Â Â Â Â Â Â Â eprintk("Verification is ignored %x\n",
> cmnd_itt(req));
>
> Â Â Â Â Â Â Â Â set_offset_and_length(req->lun, req_hdr->scb,
> &offset, &length);
> - Â Â Â Â Â Â Â if (cmnd_write_size(req) != length)
> - Â Â Â Â Â Â Â Â Â Â Â eprintk("%x %u %u\n", cmnd_itt(req),
> cmnd_write_size(req), length);
>
> Â Â Â Â Â Â Â Â req->tio = tio_alloc(get_pgcnt(length, offset));
> Â Â Â Â Â Â Â Â tio_set(req->tio, length, offset);
> @@ -1077,6 +1077,17 @@
> Â Â Â Â Â Â Â Â Â Â Â Â if (cmnd_recv_pdu(conn, req->tio, 0,
> req->pdu.datasize) < 0)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â assert(0);
> Â Â Â Â Â Â Â Â }
> +
> + Â Â Â Â Â Â Â /* WRITE_SAME_16 always have 1 data size, we pad
> the first page
> + Â Â Â Â Â Â Â Â Â to the rest of the pages. */
> + Â Â Â Â Â Â Â if (req_hdr->scb[0] == WRITE_SAME_16) {
> + Â Â Â Â Â Â Â Â Â Â Â tio_pad(req->tio, 0);
> + Â Â Â Â Â Â Â } else {
> + Â Â Â Â Â Â Â Â Â Â Â if (cmnd_write_size(req) != length) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â eprintk("%x %u %u\n",
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cmnd_itt(req),
> cmnd_write_size(req), length);
> + Â Â Â Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â break;
> Â Â Â Â }
> Â Â Â Â error:
> Index: kernel/iscsi.h
> ===================================================================
> --- kernel/iscsi.h Â Â Â (revision 470)
> +++ kernel/iscsi.h Â Â Â (working copy)

> @@ -414,6 +414,7 @@
> extern void tio_get(struct tio *);
> extern void tio_put(struct tio *);
> extern void tio_set(struct tio *, u32, loff_t);
> +extern void tio_pad(struct tio *, int);
> extern int tio_read(struct iet_volume *, struct tio *);
> extern int tio_write(struct iet_volume *, struct tio *);
> extern int tio_sync(struct iet_volume *, struct tio *);
> Index: kernel/tio.c
> ===================================================================

> --- kernel/tio.c Â Â Â Â (revision 470)
> +++ kernel/tio.c Â Â Â Â (working copy)
> @@ -124,6 +124,21 @@
> Â Â Â Â tio->size = size;

> }
>
> +void tio_pad(struct tio *tio, int idx)
> +{

> + Â Â Â int i;
> + Â Â Â u8 *src, *dst;
> +
> + Â Â Â assert(idx <= tio->pg_cnt - 1);
> + Â Â Â assert(tio->pvec[idx]);
> +
> + Â Â Â src = page_address(tio->pvec[idx]);
> + Â Â Â for (i = idx + 1; i < tio->pg_cnt; i++) {
> + Â Â Â Â Â Â Â dst = page_address(tio->pvec[i]);
> + Â Â Â Â Â Â Â memcpy(dst, src, PAGE_SIZE);
> + Â Â Â }

> +}
> +
> int tio_read(struct iet_volume *lu, struct tio *tio)
> {

> Â Â Â Â struct iotype *iot = lu->iotype;

> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> http://p.sf.net/sfu/sfd2d-msazure
> _______________________________________________
> Iscsitarget-devel mailing list
> Iscsitar...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Iscsitarget-devel mailing list
Iscsitar...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

Yucong Sun (叶雨飞)

unread,

Mar 24, 2012, 3:27:13 PM3/24/12

to Pasi Kärkkäinen, iscsitarget-devel

looks like all the tabs are corrupted , try this.

On Sat, Mar 24, 2012 at 1:08 AM, Pasi Kärkkäinen <pa...@iki.fi> wrote:

On Sat, Mar 24, 2012 at 12:43:03AM -0700, Yucong Sun (?????????) wrote:

> Hi, here's my first take on implementing WRITE_SAME_16 ,Ā
> which always has data size 1 (if I understand spec correctly),Ā
> below is the untested patch to verify that I'm in the correct direction,Ā

> Thanks.
>

Hello,

Thanks a lot for the VAAI work!

> sunyc@throne:~/iscsitarget/trunk$ cat write_same_16.patch
> Index: kernel/target_disk.c
> ===================================================================

> --- kernel/target_disk.c Ā Ā Ā Ā (revision 470)
> +++ kernel/target_disk.c Ā Ā Ā Ā (working copy)
> @@ -548,6 +548,7 @@
> Ā Ā Ā Ā case WRITE_10:
> Ā Ā Ā Ā case WRITE_16:
> Ā Ā Ā Ā case WRITE_VERIFY:
> + Ā Ā Ā case WRITE_SAME_16:
> Ā Ā Ā Ā Ā Ā Ā Ā send_scsi_rsp(cmnd, build_write_response);
> Ā Ā Ā Ā Ā Ā Ā Ā break;
> Ā Ā Ā Ā case SYNCHRONIZE_CACHE:

.. but this patch seems totally corrupted, at least for me..

-- Pasi

> Index: kernel/volume.c
> ===================================================================

> --- kernel/volume.c Ā Ā (revision 470)
> +++ kernel/volume.c Ā Ā (working copy)
> @@ -445,6 +445,7 @@
> Ā Ā Ā Ā case WRITE_12:
> Ā Ā Ā Ā case WRITE_16:
> Ā Ā Ā Ā case WRITE_VERIFY:
> + Ā Ā Ā case WRITE_SAME_16:
> Ā Ā Ā Ā case SYNCHRONIZE_CACHE:
> Ā Ā Ā Ā Ā Ā Ā Ā if (write_excl || excl_access)
> Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā err = -EBUSY;
> Index: kernel/iscsi.c
> ===================================================================
> --- kernel/iscsi.c Ā Ā Ā (revision 470)
> +++ kernel/iscsi.c Ā Ā Ā (working copy)
> @@ -777,6 +777,7 @@
> Ā Ā Ā Ā Ā Ā Ā Ā break;
> Ā Ā Ā Ā case READ_16:
> Ā Ā Ā Ā case WRITE_16:
> + Ā Ā Ā case WRITE_SAME_16:
> Ā Ā Ā Ā Ā Ā Ā Ā *off = (u64)cmd[2] << 56 | (u64)cmd[3] << 48 |
> Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā (u64)cmd[4] << 40 | (u64)cmd[5] << 32 |
> Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā (u64)cmd[6] << 24 | (u64)cmd[7] << 16 |
> @@ -1039,6 +1040,7 @@
> Ā Ā Ā Ā case WRITE_10:
> Ā Ā Ā Ā case WRITE_16:
> Ā Ā Ā Ā case WRITE_VERIFY:
> + Ā Ā Ā case WRITE_SAME_16:
> Ā Ā Ā Ā {
> Ā Ā Ā Ā Ā Ā Ā Ā struct iscsi_sess_param *param =
> &conn->session->param;
> Ā Ā Ā Ā Ā Ā Ā Ā loff_t offset;
> @@ -1067,8 +1069,6 @@
> Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā eprintk("Verification is ignored %x\n",
> cmnd_itt(req));
>
> Ā Ā Ā Ā Ā Ā Ā Ā set_offset_and_length(req->lun, req_hdr->scb,
> &offset, &length);
> - Ā Ā Ā Ā Ā Ā Ā if (cmnd_write_size(req) != length)
> - Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā eprintk("%x %u %u\n", cmnd_itt(req),
> cmnd_write_size(req), length);
>
> Ā Ā Ā Ā Ā Ā Ā Ā req->tio = tio_alloc(get_pgcnt(length, offset));
> Ā Ā Ā Ā Ā Ā Ā Ā tio_set(req->tio, length, offset);
> @@ -1077,6 +1077,17 @@
> Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā if (cmnd_recv_pdu(conn, req->tio, 0,
> req->pdu.datasize) < 0)
> Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā assert(0);
> Ā Ā Ā Ā Ā Ā Ā Ā }
> +
> + Ā Ā Ā Ā Ā Ā Ā /* WRITE_SAME_16 always have 1 data size, we pad
> the first page
> + Ā Ā Ā Ā Ā Ā Ā Ā Ā to the rest of the pages. */
> + Ā Ā Ā Ā Ā Ā Ā if (req_hdr->scb[0] == WRITE_SAME_16) {
> + Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā tio_pad(req->tio, 0);
> + Ā Ā Ā Ā Ā Ā Ā } else {
> + Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā if (cmnd_write_size(req) != length) {
> + Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā eprintk("%x %u %u\n",
> + Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā cmnd_itt(req),
> cmnd_write_size(req), length);
> + Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā Ā }
> + Ā Ā Ā Ā Ā Ā Ā }
> Ā Ā Ā Ā Ā Ā Ā Ā break;
> Ā Ā Ā Ā }
> Ā Ā Ā Ā error:
> Index: kernel/iscsi.h
> ===================================================================
> --- kernel/iscsi.h Ā Ā Ā (revision 470)
> +++ kernel/iscsi.h Ā Ā Ā (working copy)

> @@ -414,6 +414,7 @@
> extern void tio_get(struct tio *);
> extern void tio_put(struct tio *);
> extern void tio_set(struct tio *, u32, loff_t);
> +extern void tio_pad(struct tio *, int);
> extern int tio_read(struct iet_volume *, struct tio *);
> extern int tio_write(struct iet_volume *, struct tio *);
> extern int tio_sync(struct iet_volume *, struct tio *);
> Index: kernel/tio.c
> ===================================================================

> --- kernel/tio.c Ā Ā Ā Ā (revision 470)
> +++ kernel/tio.c Ā Ā Ā Ā (working copy)
> @@ -124,6 +124,21 @@
> Ā Ā Ā Ā tio->size = size;

> }
>
> +void tio_pad(struct tio *tio, int idx)
> +{

> + Ā Ā Ā int i;
> + Ā Ā Ā u8 *src, *dst;
> +
> + Ā Ā Ā assert(idx <= tio->pg_cnt - 1);
> + Ā Ā Ā assert(tio->pvec[idx]);
> +
> + Ā Ā Ā src = page_address(tio->pvec[idx]);
> + Ā Ā Ā for (i = idx + 1; i < tio->pg_cnt; i++) {
> + Ā Ā Ā Ā Ā Ā Ā dst = page_address(tio->pvec[i]);
> + Ā Ā Ā Ā Ā Ā Ā memcpy(dst, src, PAGE_SIZE);
> + Ā Ā Ā }

> +}
> +
> int tio_read(struct iet_volume *lu, struct tio *tio)
> {

> Ā Ā Ā Ā struct iotype *iot = lu->iotype;

write_same_16_patch.txt

Yucong Sun (叶雨飞)

unread,

Mar 24, 2012, 10:09:41 PM3/24/12

to Pasi Kärkkäinen, iscsitarget-devel

Actually I think this tio_pad approach would only work if PAGE_SIZE > 512 (which is pdu->datasize ) , so I might have to come up with something else.

Cheers.

Yucong Sun (叶雨飞)

unread,

Mar 25, 2012, 1:49:47 AM3/25/12

to Pasi Kärkkäinen, iscsitarget-devel

Here's a updated version, and it is tested.

Creating a 4M eager zero disk with this patch will result in 4 command each with 1M length and 512b pdu,

Mar 24 22:44:17 localhost kernel: [ 4376.047727] iscsi_trgt: scsi_cmnd_start(1090) L:1048576 O:1011875840 PDU:512

Mar 24 22:44:17 localhost kernel: [ 4376.048873] iscsi_trgt: scsi_cmnd_start(1090) L:1048576 O:1012924416 PDU:512

Mar 24 22:44:17 localhost kernel: [ 4376.050082] iscsi_trgt: scsi_cmnd_start(1090) L:1048576 O:1013972992 PDU:512

Mar 24 22:44:17 localhost kernel: [ 4376.051424] iscsi_trgt: scsi_cmnd_start(1090) L:1048576 O:1015021568 PDU:512

I've also verified the resulting disk is same (all 0) by creating a disk using iet with 0x93 support and without 0x93 support, they are all all-0 disks and sha1sum is same too.

write_same_16_patch.txt

Yucong Sun (叶雨飞)

unread,

Mar 25, 2012, 3:14:36 AM3/25/12

to Ross S. W. Walker, iscsitarget-devel

Hi Ross,

can you help to take a look? I had a few more test where 4M disk works as epxected, 32M disk does shows some garbage after about 16M , I think it's due to tio->offset and tio->idx etc, but I can't figure out exactly where.

Thanks.

Reply all

Reply to author

Forward

[Iscsitarget-devel] [DRAFT] VAAI patch 1: implement WRITE_SAME_16

Yucong Sun (叶雨飞)

Pasi Kärkkäinen

Yucong Sun (叶雨飞)

Yucong Sun (叶雨飞)

Yucong Sun (叶雨飞)

Yucong Sun (叶雨飞)