From: hooanon05@yahoo.co.jp Subject: sendfile method on read operation To: iscsitarget-devel@lists.sourceforge.net Date: Thu, 28 Apr 2005 21:46:39 +0900 Hi, all. On read operation, current iet implementation follows the below steps. this is very similar to ordinary applications. - allocate pages - call generic_file_read() and the pages are filled. - send back to the initiator - free the allocated pages i am afraid that the memory consumption and the time to allocate/free pages, since generic_file_read() uses the buffer cache. it must be reduced. and at last, i hacked iet and implemented the sendfile style. i hope this patch will improve the performance of iet and the memory usage. and i want someone whoever can do the benchmark test about the read operation, and report here, since my pc's are very old and slow and another point(pcmcia nic) got be the bottleneck. Junjiro Okajima ---------------------------------------------------------------------- Index: kernel/target_disk.c =================================================================== RCS file: /ext1/iscsi/repository/iet-0.4.7/kernel/target_disk.c,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- kernel/target_disk.c 28 Apr 2005 05:01:46 -0000 1.1 +++ kernel/target_disk.c 28 Apr 2005 12:13:17 -0000 1.2 @@ -11,6 +11,7 @@ #include "iscsi.h" #include "iscsi_dbg.h" +#include "file-io.h" static int insert_disconnect_pg(u8 *ptr) { @@ -84,7 +85,7 @@ pcode = req->scb[2] & 0x3f; assert(!tio); - tio = cmnd->tio = tio_alloc(1); + tio = cmnd->tio = tio_alloc(1, 1); data = page_address(tio->pvec[0]); assert(data); clear_page(data); @@ -151,7 +152,7 @@ return err; assert(!tio); - tio = cmnd->tio = tio_alloc(1); + tio = cmnd->tio = tio_alloc(1, 1); data = page_address(tio->pvec[0]); assert(data); clear_page(data); @@ -232,7 +233,7 @@ size = min(size & ~(8 - 1), len + 8); assert(!tio); - tio = cmnd->tio = tio_alloc(get_pgcnt(size, 0)); + tio = cmnd->tio = tio_alloc(get_pgcnt(size, 0), 1); tio_set(tio, size, 0); data = page_address(tio->pvec[idx]); @@ -263,7 +264,7 @@ u32 *data; assert(!tio); - tio = cmnd->tio = tio_alloc(1); + tio = cmnd->tio = tio_alloc(1, 1); data = page_address(tio->pvec[0]); assert(data); clear_page(data); @@ -282,7 +283,7 @@ u8 *data; assert(!tio); - tio = cmnd->tio = tio_alloc(1); + tio = cmnd->tio = tio_alloc(1, 1); data = page_address(tio->pvec[0]); assert(data); memset(data, 0, 18); @@ -303,7 +304,7 @@ /* assert((req->scb[1] & 0x1f) == 0x10); */ assert(!tio); - tio = cmnd->tio = tio_alloc(1); + tio = cmnd->tio = tio_alloc(1, 1); data = page_address(tio->pvec[0]); assert(data); clear_page(data); @@ -315,15 +316,79 @@ return 0; } +struct send_desc { + struct tio *tio; + int i; +}; + +static int +send_actor(read_descriptor_t *desc, struct page *page, + unsigned long offset, unsigned long size) +{ + unsigned long count = desc->count; + struct send_desc *a = desc->arg.data; + struct tio *tio = a->tio; + + //dprintk(D_MMCR, "size %lu, count %lu\n", size, count); + if (size > count) size = count; + tio->pvec[a->i++] = page; + desc->count = count - size; + desc->written += size; + return size; +} + static int build_read_response(struct iscsi_cmnd *cmnd) { struct tio *tio = cmnd->tio; + struct fileio_data *fileio_data; + struct file *filp; + ssize_t written, retval; + loff_t offset; + int count, ret; + struct send_desc a; assert(tio); assert(cmnd->lun); - tio_read(cmnd->lun, tio); - return 0; + fileio_data = cmnd->lun->private; + filp = fileio_data->filp; + + if (!filp->f_op->sendfile) { + tio_add_pages(tio, tio->need_pages); + tio_read(cmnd->lun, tio); + return 0; + } + + ret = -1; + offset = tio->offset; + offset += (loff_t) tio->idx << PAGE_CACHE_SHIFT; + retval = rw_verify_area(READ, filp, &offset, tio->size); + if (retval) goto err; + retval = security_file_permission (filp, MAY_READ); + if (retval) goto err; + + count = tio->need_pages*sizeof(struct page *); + tio->pvec = kmalloc(count, GFP_KERNEL); + if (!tio->pvec) goto err; + memset(tio->pvec, 0, count); + + a.tio = tio; + a.i = 0; + written = filp->f_op->sendfile(filp, &offset, tio->size, + send_actor, &a); + if (written != tio->size) { + //dprintk(D_MMCR, "written %u, tio->size %u\n", written, tio->size); + kfree(tio->pvec); + tio->pvec = NULL; + goto err; + } + ret = 0; + err: + tio->pg_cnt = 0; + //memset(tio->pvec, 0, count); + + return ret; + } static int build_write_response(struct iscsi_cmnd *cmnd) Index: kernel/tio.c =================================================================== RCS file: /ext1/iscsi/repository/iet-0.4.7/kernel/tio.c,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- kernel/tio.c 28 Apr 2005 05:01:46 -0000 1.1 +++ kernel/tio.c 28 Apr 2005 12:13:17 -0000 1.2 @@ -8,7 +8,7 @@ #include "iscsi_dbg.h" #include "iotype.h" -static int tio_add_pages(struct tio *tio, int count) +int tio_add_pages(struct tio *tio, int count) { int i; struct page *page; @@ -38,7 +38,7 @@ static kmem_cache_t *tio_cache; -struct tio *tio_alloc(int count) +struct tio *tio_alloc(int count, int add_pages_now) { struct tio *tio; @@ -52,8 +52,10 @@ atomic_set(&tio->count, 1); - if (count) + if (count && add_pages_now) tio_add_pages(tio, count); + else + tio->need_pages = count; return tio; } Index: kernel/file-io.c =================================================================== RCS file: /ext1/iscsi/repository/iet-0.4.7/kernel/file-io.c,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- kernel/file-io.c 28 Apr 2005 05:01:46 -0000 1.1 +++ kernel/file-io.c 28 Apr 2005 12:13:17 -0000 1.2 @@ -10,10 +10,7 @@ #include "iscsi.h" #include "iscsi_dbg.h" #include "iotype.h" - -struct fileio_data { - struct file *filp; -}; +#include "file-io.h" int fileio_make_request(struct iet_volume *lu, struct tio *tio, int rw) { Index: kernel/file-io.h =================================================================== RCS file: kernel/file-io.h diff -N kernel/file-io.h --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ kernel/file-io.h 28 Apr 2005 12:13:17 -0000 1.1 @@ -0,0 +1,9 @@ + +#ifndef file_io_h +#define file_io_h + +struct fileio_data { + struct file *filp; +}; + +#endif //file_io_h Index: kernel/iscsi.c =================================================================== RCS file: /ext1/iscsi/repository/iet-0.4.7/kernel/iscsi.c,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- kernel/iscsi.c 28 Apr 2005 05:01:46 -0000 1.1 +++ kernel/iscsi.c 28 Apr 2005 12:13:17 -0000 1.2 @@ -354,7 +355,7 @@ rsp_hdr->cmd_status = SAM_STAT_CHECK_CONDITION; rsp_hdr->itt = cmnd_hdr(req)->itt; - tio = rsp->tio = tio_alloc(1); + tio = rsp->tio = tio_alloc(1, 1); sense = (struct iscsi_sense_data *) page_address(tio->pvec[0]); assert(sense); clear_page(sense); @@ -444,7 +445,7 @@ if (tio) assert(tio->pg_cnt > 0); else - tio = cmnd->tio = tio_alloc(1); + tio = cmnd->tio = tio_alloc(1, 1); addr = page_address(tio->pvec[0]); assert(addr); @@ -475,7 +476,7 @@ rsp_hdr->ffffffff = ISCSI_RESERVED_TAG; rsp_hdr->reason = reason; - rsp->tio = tio = tio_alloc(1); + rsp->tio = tio = tio_alloc(1, 1); addr = page_address(tio->pvec[0]); clear_page(addr); memcpy(addr, &req->pdu.bhs, sizeof(struct iscsi_hdr)); @@ -837,7 +843,7 @@ int pg_cnt = get_pgcnt(size, 0); assert(pg_cnt < ISCSI_CONN_IOV_MAX); - cmnd->tio = tio = tio_alloc(pg_cnt); + cmnd->tio = tio = tio_alloc(pg_cnt, 1); tio_set(tio, size, 0); for (i = 0; i < pg_cnt; i++) { @@ -939,7 +957,7 @@ } set_offset_and_length(req->lun, req_hdr->scb, &offset, &length); - req->tio = tio_alloc(get_pgcnt(length, offset)); + req->tio = tio_alloc(get_pgcnt(length, offset), 0); // do not allocate pages here. tio_set(req->tio, length, offset); break; } @@ -968,7 +987,7 @@ eprintk("Verification is ignored %x\n", cmnd_itt(req)); set_offset_and_length(req->lun, req_hdr->scb, &offset, &length); - req->tio = tio_alloc(get_pgcnt(length, offset)); + req->tio = tio_alloc(get_pgcnt(length, offset), 1); tio_set(req->tio, length, offset); if (req->pdu.datasize) { Index: kernel/iscsi.h =================================================================== RCS file: /ext1/iscsi/repository/iet-0.4.7/kernel/iscsi.h,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- kernel/iscsi.h 28 Apr 2005 05:01:46 -0000 1.1 +++ kernel/iscsi.h 28 Apr 2005 12:13:17 -0000 1.2 @@ -37,6 +37,7 @@ struct tio { u32 pg_cnt; + u32 need_pages; pgoff_t idx; u32 offset; @@ -304,13 +310,14 @@ /* tio.c */ extern int tio_init(void); extern void tio_exit(void); -extern struct tio *tio_alloc(int); +extern struct tio *tio_alloc(int, int); extern void tio_get(struct tio *); extern void tio_put(struct tio *); extern void tio_set(struct tio *, u32, loff_t); 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 *); +extern int tio_add_pages(struct tio *tio, int count); /* iotype.c */ extern struct iotype *get_iotype(const char *name); ---------------------------------------------------------------------- To: iscsitarget-devel@lists.sourceforge.net Subject: Re: [Iscsitarget-devel] sendfile method on read operation From: FUJITA Tomonori In-Reply-To: References: Date: Thu, 28 Apr 2005 23:29:20 +0900 From: hooanon05@yahoo.co.jp Subject: [Iscsitarget-devel] sendfile method on read operation Date: Thu, 28 Apr 2005 21:46:39 +0900 > On read operation, current iet implementation follows the below steps. > this is very similar to ordinary applications. > - allocate pages > - call generic_file_read() and the pages are filled. > - send back to the initiator > - free the allocated pages > > i am afraid that the memory consumption and the time to allocate/free > pages, since generic_file_read() uses the buffer cache. it must be > reduced. I wrote the sendpage patch myself long time ago. http://zaal.org/iscsi/iet/misc/read-sendpage.diff I did not merge it because it does not always work (e.g. the old block I/O module and an I/O module like UNH DISKIO). I want the code to handle this cleanly. I plan to to implement after releasing the next major version. > and i want someone whoever can do the benchmark test about the read > operation, and report here, since my pc's are very old and slow and > another point(pcmcia nic) got be the bottleneck. I don't think that it makes much difference with 1GE. ---------------------------------------------------------------------- From: hooanon05@yahoo.co.jp Subject: Re: [Iscsitarget-devel] sendfile method on read operation To: iscsitarget-devel@lists.sourceforge.net References: Date: Fri, 29 Apr 2005 11:48:08 +0900 FUJITA Tomonori: > I wrote the sendpage patch myself long time ago. > > http://zaal.org/iscsi/iet/misc/read-sendpage.diff > > I did not merge it because it does not always work (e.g. the old block > I/O module and an I/O module like UNH DISKIO). i didnt know that you already have tried. does 'not always work' means some of filesystem doesn not have sendfile method? > I want the code to handle this cleanly. I plan to to implement after > releasing the next major version. i believe you can handle this very well. > I don't think that it makes much difference with 1GE. with giga nic and large memry system, it will make less difference. but on small memory system, or under heavy loading(many read operations), iet will be able to run brighter with sendfile method, i hope. Junjiro Okajima ---------------------------------------------------------------------- Subject: Re: [Iscsitarget-devel] sendfile method on read operation From: Arne Redlich To: iscsitarget-devel@lists.sourceforge.net In-Reply-To: References: Date: Fri, 29 Apr 2005 07:16:40 +0200 Am Donnerstag, den 28.04.2005, 21:46 +0900 schrieb hooanon05@yahoo.co.jp: > Hi, all. > On read operation, current iet implementation follows the below steps. > this is very similar to ordinary applications. > - allocate pages > - call generic_file_read() and the pages are filled. > - send back to the initiator > - free the allocated pages > > i am afraid that the memory consumption and the time to allocate/free > pages, since generic_file_read() uses the buffer cache. it must be > reduced. > and at last, i hacked iet and implemented the sendfile style. i hope > this patch will improve the performance of iet and the memory usage. > > and i want someone whoever can do the benchmark test about the read > operation, and report here, since my pc's are very old and slow and > another point(pcmcia nic) got be the bottleneck. > > > Junjiro Okajima > Just had a glimpse at your patch - it seems it will break (data) digest support, won't it? Arne -- Arne Redlich Xiranet Communications GmbH ---------------------------------------------------------------------- From: hooanon05@yahoo.co.jp Subject: Re: [Iscsitarget-devel] sendfile method on read operation To: iscsitarget-devel@lists.sourceforge.net References: Date: Fri, 29 Apr 2005 16:23:42 +0900 Arne Redlich: > Just had a glimpse at your patch - it seems it will break (data) digest > support, won't it? yes, i forgot about digest since i dont use it. i guess we can use current(application like) read style when the data digest is set, and can check the ddigest_type member in struct iscsi_conn before calling sendfile method. i also forgot one thing about linux-2.6.11.7. it is required to patch the source in order to export the function rw_verify_area. it is exported in linux-2.6.10, but linux-2.6.11.7 doesnt. Simply add the next line to linux-2.6.11.7/fs/read_write.c. EXPORT_SYMBOL(rw_verify_area);