* [PATCH] ps3rom: fix wrong resid calculation bug @ 2008-02-24 23:25 FUJITA Tomonori 2008-02-25 13:22 ` Geert Uytterhoeven 0 siblings, 1 reply; 6+ messages in thread From: FUJITA Tomonori @ 2008-02-24 23:25 UTC (permalink / raw) To: linux-scsi; +Cc: tomof, FUJITA Tomonori, Geert Uytterhoeven, James Bottomley sg driver rounds up the length in struct scatterlist to be a multiple of 512 in some conditions. So LLDs can't use the data length in a sg list to calculate residual. Instead, the length in struct scsi_cmnd should be used. 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> --- drivers/scsi/ps3rom.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c index 0cd614a..6944bda 100644 --- a/drivers/scsi/ps3rom.c +++ b/drivers/scsi/ps3rom.c @@ -124,7 +124,7 @@ static int fill_from_dev_buffer(struct scsi_cmnd *cmd, const void *buf) } req_len += sgpnt->length; } - scsi_set_resid(cmd, req_len - act_len); + scsi_set_resid(cmd, scsi_bufflen(cmd) - act_len); return 0; } -- 1.5.3.7 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ps3rom: fix wrong resid calculation bug 2008-02-24 23:25 [PATCH] ps3rom: fix wrong resid calculation bug FUJITA Tomonori @ 2008-02-25 13:22 ` Geert Uytterhoeven 2008-02-25 13:23 ` [PATCH 1/2] " Geert Uytterhoeven 2008-02-25 13:24 ` [PATCH 2/2] ps3rom: Simplify fill_from_dev_buffer() Geert Uytterhoeven 0 siblings, 2 replies; 6+ messages in thread From: Geert Uytterhoeven @ 2008-02-25 13:22 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: linux-scsi, tomof, James Bottomley, Geert Uytterhoeven [-- Attachment #1: Type: TEXT/PLAIN, Size: 1422 bytes --] On Sun, 24 Feb 2008, FUJITA Tomonori wrote: > sg driver rounds up the length in struct scatterlist to be a multiple > of 512 in some conditions. So LLDs can't use the data length in a sg > list to calculate residual. Instead, the length in struct scsi_cmnd > should be used. Thanks! > --- a/drivers/scsi/ps3rom.c > +++ b/drivers/scsi/ps3rom.c > @@ -124,7 +124,7 @@ static int fill_from_dev_buffer(struct scsi_cmnd *cmd, const void *buf) > } > req_len += sgpnt->length; > } > - scsi_set_resid(cmd, req_len - act_len); > + scsi_set_resid(cmd, scsi_bufflen(cmd) - act_len); > return 0; > } My 2 comments: - The variable buflen already contains scsi_bufflen(cmd), - We no longer need to calculate the data length of the whole scatterlist. I'll follow up to this email with a replacement patch and an addendum patch. With kind regards, Geert Uytterhoeven Software Architect Sony Network and Software Technology Center Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone: +32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: Geert.Uytterhoeven@sonycom.com Internet: http://www.sony-europe.com/ Sony Network and Software Technology Center Europe A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] ps3rom: fix wrong resid calculation bug 2008-02-25 13:22 ` Geert Uytterhoeven @ 2008-02-25 13:23 ` Geert Uytterhoeven 2008-02-25 13:24 ` [PATCH 2/2] ps3rom: Simplify fill_from_dev_buffer() Geert Uytterhoeven 1 sibling, 0 replies; 6+ messages in thread From: Geert Uytterhoeven @ 2008-02-25 13:23 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: linux-scsi, tomof, James Bottomley, Geert Uytterhoeven [-- Attachment #1: Type: TEXT/PLAIN, Size: 1465 bytes --] From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> sg driver rounds up the length in struct scatterlist to be a multiple of 512 in some conditions. So LLDs can't use the data length in a sg list to calculate residual. Instead, the length in struct scsi_cmnd should be used. [Geert: the variable buflen already contains scsi_bufflen(cmd)] Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> Cc: James Bottomley <James.Bottomley@HansenPartnership.com> --- drivers/scsi/ps3rom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/scsi/ps3rom.c +++ b/drivers/scsi/ps3rom.c @@ -124,7 +124,7 @@ static int fill_from_dev_buffer(struct s } req_len += sgpnt->length; } - scsi_set_resid(cmd, req_len - act_len); + scsi_set_resid(cmd, buflen - act_len); return 0; } With kind regards, Geert Uytterhoeven Software Architect Sony Network and Software Technology Center Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone: +32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: Geert.Uytterhoeven@sonycom.com Internet: http://www.sony-europe.com/ Sony Network and Software Technology Center Europe A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ps3rom: Simplify fill_from_dev_buffer() 2008-02-25 13:22 ` Geert Uytterhoeven 2008-02-25 13:23 ` [PATCH 1/2] " Geert Uytterhoeven @ 2008-02-25 13:24 ` Geert Uytterhoeven 2008-02-25 15:39 ` FUJITA Tomonori 1 sibling, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2008-02-25 13:24 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: linux-scsi, tomof, James Bottomley, Geert Uytterhoeven [-- Attachment #1: Type: TEXT/PLAIN, Size: 2408 bytes --] Subject: [PATCH] ps3rom: Simplify fill_from_dev_buffer() As we no longer need to calculate the data length of the whole scatterlist, we can abort the loop earlier and coalesce req_len and act_len into one variable, making fill_from_dev_buffer() more similar to fetch_to_dev_buffer(). Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> --- drivers/scsi/ps3rom.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) --- a/drivers/scsi/ps3rom.c +++ b/drivers/scsi/ps3rom.c @@ -95,7 +95,7 @@ static int ps3rom_slave_configure(struct */ static int fill_from_dev_buffer(struct scsi_cmnd *cmd, const void *buf) { - int k, req_len, act_len, len, active; + int k, req_len, len, fin; void *kaddr; struct scatterlist *sgpnt; unsigned int buflen; @@ -107,24 +107,22 @@ static int fill_from_dev_buffer(struct s if (!scsi_sglist(cmd)) return -1; - active = 1; - req_len = act_len = 0; + req_len = fin = 0; scsi_for_each_sg(cmd, sgpnt, scsi_sg_count(cmd), k) { - if (active) { - kaddr = kmap_atomic(sg_page(sgpnt), KM_IRQ0); - len = sgpnt->length; - if ((req_len + len) > buflen) { - active = 0; - len = buflen - req_len; - } - memcpy(kaddr + sgpnt->offset, buf + req_len, len); - flush_kernel_dcache_page(sg_page(sgpnt)); - kunmap_atomic(kaddr, KM_IRQ0); - act_len += len; + kaddr = kmap_atomic(sg_page(sgpnt), KM_IRQ0); + len = sgpnt->length; + if ((req_len + len) > buflen) { + len = buflen - req_len; + fin = 1; } - req_len += sgpnt->length; + memcpy(kaddr + sgpnt->offset, buf + req_len, len); + flush_kernel_dcache_page(sg_page(sgpnt)); + kunmap_atomic(kaddr, KM_IRQ0); + req_len += len; + if (fin) + break; } - scsi_set_resid(cmd, buflen - act_len); + scsi_set_resid(cmd, buflen - req_len); return 0; } With kind regards, Geert Uytterhoeven Software Architect Sony Network and Software Technology Center Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone: +32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: Geert.Uytterhoeven@sonycom.com Internet: http://www.sony-europe.com/ Sony Network and Software Technology Center Europe A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ps3rom: Simplify fill_from_dev_buffer() 2008-02-25 13:24 ` [PATCH 2/2] ps3rom: Simplify fill_from_dev_buffer() Geert Uytterhoeven @ 2008-02-25 15:39 ` FUJITA Tomonori 2008-02-25 16:19 ` Geert Uytterhoeven 0 siblings, 1 reply; 6+ messages in thread From: FUJITA Tomonori @ 2008-02-25 15:39 UTC (permalink / raw) To: Geert.Uytterhoeven; +Cc: fujita.tomonori, linux-scsi, tomof, James.Bottomley On Mon, 25 Feb 2008 14:24:31 +0100 (CET) Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote: > Subject: [PATCH] ps3rom: Simplify fill_from_dev_buffer() > > As we no longer need to calculate the data length of the whole scatterlist, > we can abort the loop earlier and coalesce req_len and act_len into one > variable, making fill_from_dev_buffer() more similar to fetch_to_dev_buffer(). I'll add new APIs to copy data between a sg list and a buffer soon after cleaning up (and fixing) some drivers on this area. I plan to remove fill_from_dev_buffer and fetch_to_dev_buffer in ps3rom. As you know, they are same functions that scsi_debug uses. There are other drivers that need similar functions. I expect my ps3rom patch to be applied to the scsi-fixes tree so I didn't change much as you did in this patch. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ps3rom: Simplify fill_from_dev_buffer() 2008-02-25 15:39 ` FUJITA Tomonori @ 2008-02-25 16:19 ` Geert Uytterhoeven 0 siblings, 0 replies; 6+ messages in thread From: Geert Uytterhoeven @ 2008-02-25 16:19 UTC (permalink / raw) To: FUJITA Tomonori Cc: fujita.tomonori, linux-scsi, James E.J. Bottomley, Geert Uytterhoeven [-- Attachment #1: Type: TEXT/PLAIN, Size: 1499 bytes --] On Tue, 26 Feb 2008, FUJITA Tomonori wrote: > On Mon, 25 Feb 2008 14:24:31 +0100 (CET) > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote: > > > Subject: [PATCH] ps3rom: Simplify fill_from_dev_buffer() > > > > As we no longer need to calculate the data length of the whole scatterlist, > > we can abort the loop earlier and coalesce req_len and act_len into one > > variable, making fill_from_dev_buffer() more similar to fetch_to_dev_buffer(). > > I'll add new APIs to copy data between a sg list and a buffer soon > after cleaning up (and fixing) some drivers on this area. I plan to > remove fill_from_dev_buffer and fetch_to_dev_buffer in ps3rom. As you > know, they are same functions that scsi_debug uses. There are other > drivers that need similar functions. I expect my ps3rom patch to be > applied to the scsi-fixes tree so I didn't change much as you did in > this patch. IC, thanks! With kind regards, Geert Uytterhoeven Software Architect Sony Network and Software Technology Center Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone: +32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: Geert.Uytterhoeven@sonycom.com Internet: http://www.sony-europe.com/ Sony Network and Software Technology Center Europe A division of Sony Service Centre (Europe) N.V. Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium VAT BE 0413.825.160 · RPR Brussels Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-02-25 16:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-02-24 23:25 [PATCH] ps3rom: fix wrong resid calculation bug FUJITA Tomonori 2008-02-25 13:22 ` Geert Uytterhoeven 2008-02-25 13:23 ` [PATCH 1/2] " Geert Uytterhoeven 2008-02-25 13:24 ` [PATCH 2/2] ps3rom: Simplify fill_from_dev_buffer() Geert Uytterhoeven 2008-02-25 15:39 ` FUJITA Tomonori 2008-02-25 16:19 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox