* [PATCH 0/3] introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() @ 2013-06-06 12:52 Akinobu Mita 2013-06-06 12:52 ` [PATCH 1/3] lib/scatterlist: " Akinobu Mita 2013-06-06 12:52 ` [PATCH 3/3] scsi_debug: fix do_device_access() with wrap around range Akinobu Mita 0 siblings, 2 replies; 7+ messages in thread From: Akinobu Mita @ 2013-06-06 12:52 UTC (permalink / raw) To: linux-kernel, akpm Cc: Akinobu Mita, Tejun Heo, Imre Deak, Herbert Xu, David S. Miller, linux-crypto, James E.J. Bottomley, Douglas Gilbert, linux-scsi This patch set introduces sg_pcopy_from_buffer() and sg_pcopy_to_buffer(), which copy data between a linear buffer and an SG list. The only difference between sg_pcopy_{from,to}_buffer() and sg_copy_{from,to}_buffer() is an additional argument that specifies the number of bytes to skip the SG list before copying. The main reason for introducing these functions is to fix a problem in scsi_debug module. And there is a local function in crypto/talitos module, which can be replaced by sg_pcopy_to_buffer(). Akinobu Mita (3): lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() crypto: talitos: use sg_pcopy_to_buffer() scsi_debug: fix do_device_access() with wrap around range drivers/crypto/talitos.c | 60 +----------------------- drivers/scsi/scsi_debug.c | 43 ++++++++++++++--- include/linux/scatterlist.h | 5 ++ lib/scatterlist.c | 109 ++++++++++++++++++++++++++++++++++++-------- 4 files changed, 131 insertions(+), 86 deletions(-) Cc: Tejun Heo <tj@kernel.org> Cc: Imre Deak <imre.deak@intel.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: linux-crypto@vger.kernel.org Cc: "James E.J. Bottomley" <JBottomley@parallels.com> Cc: Douglas Gilbert <dgilbert@interlog.com> Cc: linux-scsi@vger.kernel.org -- 1.8.1.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() 2013-06-06 12:52 [PATCH 0/3] introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() Akinobu Mita @ 2013-06-06 12:52 ` Akinobu Mita 2013-06-06 13:12 ` Imre Deak 2013-06-06 21:00 ` Tejun Heo 2013-06-06 12:52 ` [PATCH 3/3] scsi_debug: fix do_device_access() with wrap around range Akinobu Mita 1 sibling, 2 replies; 7+ messages in thread From: Akinobu Mita @ 2013-06-06 12:52 UTC (permalink / raw) To: linux-kernel, akpm Cc: Akinobu Mita, Tejun Heo, Imre Deak, Herbert Xu, David S. Miller, linux-crypto, James E.J. Bottomley, Douglas Gilbert, linux-scsi The only difference between sg_pcopy_{from,to}_buffer() and sg_copy_{from,to}_buffer() is an additional argument that specifies the number of bytes to skip the SG list before copying. Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Cc: Tejun Heo <tj@kernel.org> Cc: Imre Deak <imre.deak@intel.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: linux-crypto@vger.kernel.org Cc: "James E.J. Bottomley" <JBottomley@parallels.com> Cc: Douglas Gilbert <dgilbert@interlog.com> Cc: linux-scsi@vger.kernel.org --- include/linux/scatterlist.h | 5 ++ lib/scatterlist.c | 109 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 94 insertions(+), 20 deletions(-) diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 5951e3f..f5dee42 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -241,6 +241,11 @@ size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen); +size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents, + void *buf, size_t buflen, off_t skip); +size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents, + void *buf, size_t buflen, off_t skip); + /* * Maximum number of entries that will be allocated in one piece, if * a list larger than this is required then chaining will be utilized. diff --git a/lib/scatterlist.c b/lib/scatterlist.c index a1cf8ca..3b40b72 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -453,6 +453,47 @@ void sg_miter_start(struct sg_mapping_iter *miter, struct scatterlist *sgl, } EXPORT_SYMBOL(sg_miter_start); +static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) +{ + if (!miter->__remaining) { + struct scatterlist *sg; + unsigned long pgoffset; + + if (!__sg_page_iter_next(&miter->piter)) + return false; + + sg = miter->piter.sg; + pgoffset = miter->piter.sg_pgoffset; + + miter->__offset = pgoffset ? 0 : sg->offset; + miter->__remaining = sg->offset + sg->length - + (pgoffset << PAGE_SHIFT) - miter->__offset; + miter->__remaining = min_t(unsigned long, miter->__remaining, + PAGE_SIZE - miter->__offset); + } + + return true; +} + +static bool sg_miter_seek(struct sg_mapping_iter *miter, off_t offset) +{ + WARN_ON(miter->addr); + + while (offset) { + off_t consumed; + + if (!sg_miter_get_next_page(miter)) + return false; + + consumed = min_t(off_t, offset, miter->__remaining); + miter->__offset += consumed; + miter->__remaining -= consumed; + offset -= consumed; + } + + return true; +} + /** * sg_miter_next - proceed mapping iterator to the next mapping * @miter: sg mapping iter to proceed @@ -478,22 +519,9 @@ bool sg_miter_next(struct sg_mapping_iter *miter) * Get to the next page if necessary. * __remaining, __offset is adjusted by sg_miter_stop */ - if (!miter->__remaining) { - struct scatterlist *sg; - unsigned long pgoffset; - - if (!__sg_page_iter_next(&miter->piter)) - return false; - - sg = miter->piter.sg; - pgoffset = miter->piter.sg_pgoffset; + if (!sg_miter_get_next_page(miter)) + return false; - miter->__offset = pgoffset ? 0 : sg->offset; - miter->__remaining = sg->offset + sg->length - - (pgoffset << PAGE_SHIFT) - miter->__offset; - miter->__remaining = min_t(unsigned long, miter->__remaining, - PAGE_SIZE - miter->__offset); - } miter->page = sg_page_iter_page(&miter->piter); miter->consumed = miter->length = miter->__remaining; @@ -552,14 +580,16 @@ EXPORT_SYMBOL(sg_miter_stop); * @nents: Number of SG entries * @buf: Where to copy from * @buflen: The number of bytes to copy - * @to_buffer: transfer direction (non zero == from an sg list to a - * buffer, 0 == from a buffer to an sg list + * @skip: Number of bytes to skip before copying + * @to_buffer: transfer direction (true == from an sg list to a + * buffer, false == from a buffer to an sg list * * Returns the number of copied bytes. * **/ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, - void *buf, size_t buflen, int to_buffer) + void *buf, size_t buflen, off_t skip, + bool to_buffer) { unsigned int offset = 0; struct sg_mapping_iter miter; @@ -573,6 +603,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, sg_miter_start(&miter, sgl, nents, sg_flags); + if (!sg_miter_seek(&miter, skip)) + return false; + local_irq_save(flags); while (sg_miter_next(&miter) && offset < buflen) { @@ -607,7 +640,7 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen) { - return sg_copy_buffer(sgl, nents, buf, buflen, 0); + return sg_copy_buffer(sgl, nents, buf, buflen, 0, false); } EXPORT_SYMBOL(sg_copy_from_buffer); @@ -624,6 +657,42 @@ EXPORT_SYMBOL(sg_copy_from_buffer); size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen) { - return sg_copy_buffer(sgl, nents, buf, buflen, 1); + return sg_copy_buffer(sgl, nents, buf, buflen, 0, true); } EXPORT_SYMBOL(sg_copy_to_buffer); + +/** + * sg_pcopy_from_buffer - Copy from a linear buffer to an SG list + * @sgl: The SG list + * @nents: Number of SG entries + * @buf: Where to copy from + * @skip: Number of bytes to skip before copying + * @buflen: The number of bytes to copy + * + * Returns the number of copied bytes. + * + **/ +size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents, + void *buf, size_t buflen, off_t skip) +{ + return sg_copy_buffer(sgl, nents, buf, buflen, skip, false); +} +EXPORT_SYMBOL(sg_pcopy_from_buffer); + +/** + * sg_pcopy_to_buffer - Copy from an SG list to a linear buffer + * @sgl: The SG list + * @nents: Number of SG entries + * @buf: Where to copy to + * @skip: Number of bytes to skip before copying + * @buflen: The number of bytes to copy + * + * Returns the number of copied bytes. + * + **/ +size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents, + void *buf, size_t buflen, off_t skip) +{ + return sg_copy_buffer(sgl, nents, buf, buflen, skip, true); +} +EXPORT_SYMBOL(sg_pcopy_to_buffer); -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() 2013-06-06 12:52 ` [PATCH 1/3] lib/scatterlist: " Akinobu Mita @ 2013-06-06 13:12 ` Imre Deak 2013-06-08 14:04 ` Akinobu Mita 2013-06-06 21:00 ` Tejun Heo 1 sibling, 1 reply; 7+ messages in thread From: Imre Deak @ 2013-06-06 13:12 UTC (permalink / raw) To: Akinobu Mita Cc: linux-kernel, akpm, Tejun Heo, Herbert Xu, David S. Miller, linux-crypto, James E.J. Bottomley, Douglas Gilbert, linux-scsi On Thu, 2013-06-06 at 21:52 +0900, Akinobu Mita wrote: > The only difference between sg_pcopy_{from,to}_buffer() and > sg_copy_{from,to}_buffer() is an additional argument that specifies > the number of bytes to skip the SG list before copying. > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Tejun Heo <tj@kernel.org> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: linux-crypto@vger.kernel.org > Cc: "James E.J. Bottomley" <JBottomley@parallels.com> > Cc: Douglas Gilbert <dgilbert@interlog.com> > Cc: linux-scsi@vger.kernel.org > --- > include/linux/scatterlist.h | 5 ++ > lib/scatterlist.c | 109 ++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 94 insertions(+), 20 deletions(-) > > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > index 5951e3f..f5dee42 100644 > --- a/include/linux/scatterlist.h > +++ b/include/linux/scatterlist.h > @@ -241,6 +241,11 @@ size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, > size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents, > void *buf, size_t buflen); > > +size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents, > + void *buf, size_t buflen, off_t skip); > +size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents, > + void *buf, size_t buflen, off_t skip); > + > /* > * Maximum number of entries that will be allocated in one piece, if > * a list larger than this is required then chaining will be utilized. > diff --git a/lib/scatterlist.c b/lib/scatterlist.c > index a1cf8ca..3b40b72 100644 > --- a/lib/scatterlist.c > +++ b/lib/scatterlist.c > @@ -453,6 +453,47 @@ void sg_miter_start(struct sg_mapping_iter *miter, struct scatterlist *sgl, > } > EXPORT_SYMBOL(sg_miter_start); > > +static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) > +{ > + if (!miter->__remaining) { > + struct scatterlist *sg; > + unsigned long pgoffset; > + > + if (!__sg_page_iter_next(&miter->piter)) > + return false; > + > + sg = miter->piter.sg; > + pgoffset = miter->piter.sg_pgoffset; > + > + miter->__offset = pgoffset ? 0 : sg->offset; > + miter->__remaining = sg->offset + sg->length - > + (pgoffset << PAGE_SHIFT) - miter->__offset; > + miter->__remaining = min_t(unsigned long, miter->__remaining, > + PAGE_SIZE - miter->__offset); > + } > + > + return true; > +} > + > +static bool sg_miter_seek(struct sg_mapping_iter *miter, off_t offset) > +{ > + WARN_ON(miter->addr); > + > + while (offset) { > + off_t consumed; > + > + if (!sg_miter_get_next_page(miter)) > + return false; > + > + consumed = min_t(off_t, offset, miter->__remaining); > + miter->__offset += consumed; > + miter->__remaining -= consumed; > + offset -= consumed; > + } > + > + return true; > +} > + > /** > * sg_miter_next - proceed mapping iterator to the next mapping > * @miter: sg mapping iter to proceed > @@ -478,22 +519,9 @@ bool sg_miter_next(struct sg_mapping_iter *miter) > * Get to the next page if necessary. > * __remaining, __offset is adjusted by sg_miter_stop > */ > - if (!miter->__remaining) { > - struct scatterlist *sg; > - unsigned long pgoffset; > - > - if (!__sg_page_iter_next(&miter->piter)) > - return false; > - > - sg = miter->piter.sg; > - pgoffset = miter->piter.sg_pgoffset; > + if (!sg_miter_get_next_page(miter)) > + return false; > > - miter->__offset = pgoffset ? 0 : sg->offset; > - miter->__remaining = sg->offset + sg->length - > - (pgoffset << PAGE_SHIFT) - miter->__offset; > - miter->__remaining = min_t(unsigned long, miter->__remaining, > - PAGE_SIZE - miter->__offset); > - } > miter->page = sg_page_iter_page(&miter->piter); > miter->consumed = miter->length = miter->__remaining; > > @@ -552,14 +580,16 @@ EXPORT_SYMBOL(sg_miter_stop); > * @nents: Number of SG entries > * @buf: Where to copy from > * @buflen: The number of bytes to copy > - * @to_buffer: transfer direction (non zero == from an sg list to a > - * buffer, 0 == from a buffer to an sg list > + * @skip: Number of bytes to skip before copying > + * @to_buffer: transfer direction (true == from an sg list to a > + * buffer, false == from a buffer to an sg list > * > * Returns the number of copied bytes. > * > **/ > static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > - void *buf, size_t buflen, int to_buffer) > + void *buf, size_t buflen, off_t skip, > + bool to_buffer) > { > unsigned int offset = 0; > struct sg_mapping_iter miter; > @@ -573,6 +603,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > > sg_miter_start(&miter, sgl, nents, sg_flags); > > + if (!sg_miter_seek(&miter, skip)) > + return false; Looks ok to me, perhaps adding the seek functionality to the mapping iterator would make things more generic and the mapping iterator more resemble the page iterator. So we'd have a new sg_miter_start_offset and call it here something like: sg_miter_start_offset(&miter, sgl, nents, sg_flags, skip); Just my 2 cents, Imre > + > local_irq_save(flags); > > while (sg_miter_next(&miter) && offset < buflen) { > @@ -607,7 +640,7 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, > size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, > void *buf, size_t buflen) > { > - return sg_copy_buffer(sgl, nents, buf, buflen, 0); > + return sg_copy_buffer(sgl, nents, buf, buflen, 0, false); > } > EXPORT_SYMBOL(sg_copy_from_buffer); > > @@ -624,6 +657,42 @@ EXPORT_SYMBOL(sg_copy_from_buffer); > size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents, > void *buf, size_t buflen) > { > - return sg_copy_buffer(sgl, nents, buf, buflen, 1); > + return sg_copy_buffer(sgl, nents, buf, buflen, 0, true); > } > EXPORT_SYMBOL(sg_copy_to_buffer); > + > +/** > + * sg_pcopy_from_buffer - Copy from a linear buffer to an SG list > + * @sgl: The SG list > + * @nents: Number of SG entries > + * @buf: Where to copy from > + * @skip: Number of bytes to skip before copying > + * @buflen: The number of bytes to copy > + * > + * Returns the number of copied bytes. > + * > + **/ > +size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents, > + void *buf, size_t buflen, off_t skip) > +{ > + return sg_copy_buffer(sgl, nents, buf, buflen, skip, false); > +} > +EXPORT_SYMBOL(sg_pcopy_from_buffer); > + > +/** > + * sg_pcopy_to_buffer - Copy from an SG list to a linear buffer > + * @sgl: The SG list > + * @nents: Number of SG entries > + * @buf: Where to copy to > + * @skip: Number of bytes to skip before copying > + * @buflen: The number of bytes to copy > + * > + * Returns the number of copied bytes. > + * > + **/ > +size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents, > + void *buf, size_t buflen, off_t skip) > +{ > + return sg_copy_buffer(sgl, nents, buf, buflen, skip, true); > +} > +EXPORT_SYMBOL(sg_pcopy_to_buffer); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() 2013-06-06 13:12 ` Imre Deak @ 2013-06-08 14:04 ` Akinobu Mita 0 siblings, 0 replies; 7+ messages in thread From: Akinobu Mita @ 2013-06-08 14:04 UTC (permalink / raw) To: imre.deak Cc: LKML, Andrew Morton, Tejun Heo, Herbert Xu, David S. Miller, linux-crypto, James E.J. Bottomley, Douglas Gilbert, linux-scsi 2013/6/6 Imre Deak <imre.deak@intel.com>: > Looks ok to me, perhaps adding the seek functionality to the mapping > iterator would make things more generic and the mapping iterator more > resemble the page iterator. So we'd have a new sg_miter_start_offset and > call it here something like: > > sg_miter_start_offset(&miter, sgl, nents, sg_flags, skip); I also thought something like this could be a new interface for sg_miter_* API, but I haven't found any places where it can be used yet. That's why I made it a local function and didn't create new interface. But putting good function comment like other sg_miter_* API for new local function is harmless and it helps someone who wants interface like this in the future. So I'll do so in next version. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() 2013-06-06 12:52 ` [PATCH 1/3] lib/scatterlist: " Akinobu Mita 2013-06-06 13:12 ` Imre Deak @ 2013-06-06 21:00 ` Tejun Heo 2013-06-08 14:28 ` Akinobu Mita 1 sibling, 1 reply; 7+ messages in thread From: Tejun Heo @ 2013-06-06 21:00 UTC (permalink / raw) To: Akinobu Mita Cc: linux-kernel, akpm, Imre Deak, Herbert Xu, David S. Miller, linux-crypto, James E.J. Bottomley, Douglas Gilbert, linux-scsi Hello, On Thu, Jun 06, 2013 at 09:52:56PM +0900, Akinobu Mita wrote: > +static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) > +{ > + if (!miter->__remaining) { > + struct scatterlist *sg; > + unsigned long pgoffset; > + > + if (!__sg_page_iter_next(&miter->piter)) > + return false; > + > + sg = miter->piter.sg; > + pgoffset = miter->piter.sg_pgoffset; > + > + miter->__offset = pgoffset ? 0 : sg->offset; > + miter->__remaining = sg->offset + sg->length - > + (pgoffset << PAGE_SHIFT) - miter->__offset; > + miter->__remaining = min_t(unsigned long, miter->__remaining, > + PAGE_SIZE - miter->__offset); > + } > + > + return true; > +} It'd be better if separating out this function is a separate patch. Mixing factoring out something and adding new stuff makes the patch a bit harder to read. > +static bool sg_miter_seek(struct sg_mapping_iter *miter, off_t offset) > +{ > + WARN_ON(miter->addr); > + > + while (offset) { > + off_t consumed; > + > + if (!sg_miter_get_next_page(miter)) > + return false; > + > + consumed = min_t(off_t, offset, miter->__remaining); > + miter->__offset += consumed; > + miter->__remaining -= consumed; > + offset -= consumed; > + } > + > + return true; > +} While I think the above should work at the beginning, what if @miter is in the middle of iteration and __remaining isn't zero? Looks good to me otherwise. Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() 2013-06-06 21:00 ` Tejun Heo @ 2013-06-08 14:28 ` Akinobu Mita 0 siblings, 0 replies; 7+ messages in thread From: Akinobu Mita @ 2013-06-08 14:28 UTC (permalink / raw) To: Tejun Heo Cc: LKML, Andrew Morton, Imre Deak, Herbert Xu, David S. Miller, linux-crypto, James E.J. Bottomley, Douglas Gilbert, linux-scsi 2013/6/7 Tejun Heo <tj@kernel.org>: > Hello, > > On Thu, Jun 06, 2013 at 09:52:56PM +0900, Akinobu Mita wrote: >> +static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) >> +{ >> + if (!miter->__remaining) { >> + struct scatterlist *sg; >> + unsigned long pgoffset; >> + >> + if (!__sg_page_iter_next(&miter->piter)) >> + return false; >> + >> + sg = miter->piter.sg; >> + pgoffset = miter->piter.sg_pgoffset; >> + >> + miter->__offset = pgoffset ? 0 : sg->offset; >> + miter->__remaining = sg->offset + sg->length - >> + (pgoffset << PAGE_SHIFT) - miter->__offset; >> + miter->__remaining = min_t(unsigned long, miter->__remaining, >> + PAGE_SIZE - miter->__offset); >> + } >> + >> + return true; >> +} > > It'd be better if separating out this function is a separate patch. > Mixing factoring out something and adding new stuff makes the patch a > bit harder to read. OK, sounds good. I'll do so in next version. But I feel sg_miter_get_next_page() is not very appropriate name, because it gets the next page only if necessary. If I can find more appropriate name, I'll rename it. >> +static bool sg_miter_seek(struct sg_mapping_iter *miter, off_t offset) >> +{ >> + WARN_ON(miter->addr); >> + >> + while (offset) { >> + off_t consumed; >> + >> + if (!sg_miter_get_next_page(miter)) >> + return false; >> + >> + consumed = min_t(off_t, offset, miter->__remaining); >> + miter->__offset += consumed; >> + miter->__remaining -= consumed; >> + offset -= consumed; >> + } >> + >> + return true; >> +} > > While I think the above should work at the beginning, what if @miter > is in the middle of iteration and __remaining isn't zero? sg_miter_seek() should work as far as it is called just after sg_miter_start(), or just after sg_miter_stop() in the middle of iteration (Although I only tested the former case). I omitted a function comment in excuse of the static function, but I should write something I said above. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] scsi_debug: fix do_device_access() with wrap around range 2013-06-06 12:52 [PATCH 0/3] introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() Akinobu Mita 2013-06-06 12:52 ` [PATCH 1/3] lib/scatterlist: " Akinobu Mita @ 2013-06-06 12:52 ` Akinobu Mita 1 sibling, 0 replies; 7+ messages in thread From: Akinobu Mita @ 2013-06-06 12:52 UTC (permalink / raw) To: linux-kernel, akpm Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert, linux-scsi do_device_access() is a function that abstracts copying SG list from/to ramdisk storage (fake_storep). It must deal with the ranges exceeding actual fake_storep size, because such ranges are valid if virtual_gb is set greater than zero, and they should be treated as fake_storep is repeatedly mirrored up to virtual size. Unfortunately, it can't deal with the range which wraps around the end of fake_storep. A wrap around range is copied by two sg_copy_{from,to}_buffer() calls, but sg_copy_{from,to}_buffer() can't copy from/to in the middle of SG list, therefore the second call can't copy correctly. This fixes it by using sg_pcopy_{from,to}_buffer() that can copy from/to the middle of SG list. Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Cc: "James E.J. Bottomley" <JBottomley@parallels.com> Cc: Douglas Gilbert <dgilbert@interlog.com> Cc: linux-scsi@vger.kernel.org --- drivers/scsi/scsi_debug.c | 43 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 21239b3..c1efaf8 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -1614,24 +1614,48 @@ static int check_device_access_params(struct sdebug_dev_info *devi, return 0; } +/* Returns number of bytes copied or -1 if error. */ static int do_device_access(struct scsi_cmnd *scmd, struct sdebug_dev_info *devi, unsigned long long lba, unsigned int num, int write) { int ret; unsigned long long block, rest = 0; - int (*func)(struct scsi_cmnd *, unsigned char *, int); + struct scsi_data_buffer *sdb; + enum dma_data_direction dir; + size_t (*func)(struct scatterlist *, unsigned int, void *, size_t, + off_t); + + if (write) { + sdb = scsi_out(scmd); + dir = DMA_TO_DEVICE; + func = sg_pcopy_to_buffer; + } else { + sdb = scsi_in(scmd); + dir = DMA_FROM_DEVICE; + func = sg_pcopy_from_buffer; + } - func = write ? fetch_to_dev_buffer : fill_from_dev_buffer; + if (!sdb->length) + return 0; + if (!(scsi_bidi_cmnd(scmd) || scmd->sc_data_direction == dir)) + return -1; block = do_div(lba, sdebug_store_sectors); if (block + num > sdebug_store_sectors) rest = block + num - sdebug_store_sectors; - ret = func(scmd, fake_storep + (block * scsi_debug_sector_size), - (num - rest) * scsi_debug_sector_size); - if (!ret && rest) - ret = func(scmd, fake_storep, rest * scsi_debug_sector_size); + ret = func(sdb->table.sgl, sdb->table.nents, + fake_storep + (block * scsi_debug_sector_size), + (num - rest) * scsi_debug_sector_size, 0); + if (ret != (num - rest) * scsi_debug_sector_size) + return ret; + + if (rest) { + ret += func(sdb->table.sgl, sdb->table.nents, + fake_storep, rest * scsi_debug_sector_size, + (num - rest) * scsi_debug_sector_size); + } return ret; } @@ -1888,7 +1912,12 @@ static int resp_read(struct scsi_cmnd *SCpnt, unsigned long long lba, read_lock_irqsave(&atomic_rw, iflags); ret = do_device_access(SCpnt, devip, lba, num, 0); read_unlock_irqrestore(&atomic_rw, iflags); - return ret; + if (ret == -1) + return DID_ERROR << 16; + + scsi_in(SCpnt)->resid = scsi_bufflen(SCpnt) - ret; + + return 0; } static void dump_sector(unsigned char *buf, int len) -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-06-08 14:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-06 12:52 [PATCH 0/3] introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer() Akinobu Mita 2013-06-06 12:52 ` [PATCH 1/3] lib/scatterlist: " Akinobu Mita 2013-06-06 13:12 ` Imre Deak 2013-06-08 14:04 ` Akinobu Mita 2013-06-06 21:00 ` Tejun Heo 2013-06-08 14:28 ` Akinobu Mita 2013-06-06 12:52 ` [PATCH 3/3] scsi_debug: fix do_device_access() with wrap around range Akinobu Mita
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).