introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()
Subsystems (3)
v2 cover introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()
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().
* Changes from v1
- Separate the change that factors out sg_miter_get_next_page() from the patch "introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()"
- Add function comment for internal function sg_miter_seek()
- Simplify the assignment of sdb->resid in fill_from_dev_buffer() in scsi_debug
Akinobu Mita (4):
lib/scatterlist: factor out sg_miter_get_next_page() from
sg_miter_next()
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 | 48 +++++++++++++---- include/linux/scatterlist.h | 5 ++ lib/scatterlist.c | 127 +++++++++++++++++++++++++++++++++++++------- 4 files changed, 150 insertions(+), 90 deletions(-)
Signature · 1 line
1.8.1.4
PATCH v2 1/4 lib/scatterlist: factor out sg_miter_get_next_page() from sg_miter_next()
This function is used to proceed page iterator to the next page if necessary, and will be used to implement the variants of sg_copy_{from,to}_buffer() later.
---
* New patch from v2
lib/scatterlist.c | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-)
lib/scatterlist.c +24 -15
@@ -453,6 +453,28 @@ 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; +} +/** * sg_miter_next - proceed mapping iterator to the next mapping * @miter: sg mapping iter to proceed
@@ -478,22 +500,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;
Signature · 1 line
1.8.1.4
Re: [PATCH v2 1/4] lib/scatterlist: factor out sg_miter_get_next_page() from sg_miter_next()
Quoted 4 lines · Akinobu Mita
This function is used to proceed page iterator to the next page if necessary, and will be used to implement the variants of sg_copy_{from,to}_buffer() later.
Quoted 9 lines
Signed-off-by Akinobu Mita <[email protected]>Cc Tejun Heo <[email protected]>Cc Imre Deak <[email protected]>Cc Herbert Xu <[email protected]>Cc "David S. Miller" <[email protected]>Cc "James E.J. Bottomley" <[email protected]>Cc Douglas Gilbert <[email protected]>
Thanks.
Signature · 1 line
tejun
PATCH v2 2/4 lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()
In reply to Akinobu MitaThe 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.
---
* Change from v2
- Separate the change that factors out sg_miter_get_next_page()
- Add function comment for internal function sg_miter_seek()
include/linux/scatterlist.h | 5 +++ lib/scatterlist.c | 88 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 88 insertions(+), 5 deletions(-)
include/linux/scatterlist.h +5 -0
@@ -244,6 +244,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.
lib/scatterlist.c +83 -5
122 lines not on this page · raw message
Signature · 1 line
1.8.1.4
Re: [PATCH v2 2/4] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()
Quoted 10 lines · Akinobu Mita
/**
+ *sg_miter_seek- reposition mapping iterator
+ * @miter: sg mapping iter to be seeked
+ * @offset: number of bytes to plus the current location
+ *
+ * Description:
+ * Sets the offset of @miter to its current location plus @offset bytes.
+ *
+ * Notes:
+ * This function must be called just aftersg_miter_start()orsg_miter_stop()
It's not gonna be hard to make this function to handle any state, right? Wouldn't it be better to do that? It's a pretty generic feature after all. Also, maybe a better name is sg_miter_skip()?
Thanks.
Signature · 1 line
tejun
Re: [PATCH v2 2/4] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()
2013/6/18 Tejun Heo <[email protected]>:
Quoted 12 lines · Akinobu Mita
Quoted 10 lines
/**
+ *sg_miter_seek- reposition mapping iterator
+ * @miter: sg mapping iter to be seeked
+ * @offset: number of bytes to plus the current location
+ *
+ * Description:
+ * Sets the offset of @miter to its current location plus @offset bytes.
+ *
+ * Notes:
+ * This function must be called just aftersg_miter_start()orsg_miter_stop()
Quoted 3 lines
It's not gonna be hard to make this function to handle any state, right? Wouldn't it be better to do that? It's a pretty generic feature after all. Also, maybe a better name is
sg_miter_skip()?
Yes. This limitation can be removed by calling sg_miter_stop() at the front of sg_miter_seek(). Also, renaming it to sg_miter_skip() sounds good.
PATCH v2 3/4 crypto: talitos: use sg_pcopy_to_buffer()
In reply to Akinobu MitaUse sg_pcopy_to_buffer() which is better than the function previously used. Because it doesn't do kmap/kunmap for skipped pages.
---
* No change from v1
drivers/crypto/talitos.c | 60 +----------------------------------------------- 1 file changed, 1 insertion(+), 59 deletions(-)
drivers/crypto/talitos.c +1 -59
@@ -1112,64 +1112,6 @@ static int sg_count(struct scatterlist *sg_list, int nbytes, bool *chained)return sg_nents; } -/** -* sg_copy_end_to_buffer - Copy end data from SG list to a linear buffer -* @sgl: The SG list -* @nents: Number of SG entries -* @buf: Where to copy to -* @buflen: The number of bytes to copy -* @skip: The number of bytes to skip before copying. -* Note: skip + buflen should equal SG total size. -* -* Returns the number of copied bytes. -* -**/ -static size_t sg_copy_end_to_buffer (struct scatterlist *sgl,unsigned int nents, -void *buf,size_t buflen,unsigned int skip) -{ -unsigned int offset =0 ; -unsigned int boffset =0 ; -struct sg_mapping_iter miter; -unsigned long flags; -unsigned int sg_flags =SG_MITER_ATOMIC ; -size_t total_buffer = buflen + skip; - - sg_flags |=SG_MITER_FROM_SG ; - -sg_miter_start (&miter, sgl, nents, sg_flags); - -local_irq_save (flags); - -while (sg_miter_next (&miter) && offset < total_buffer) { -unsigned int len; -unsigned int ignore; - -if ((offset + miter.length) > skip) { -if (offset < skip) { -/* Copy part of this segment */ - ignore = skip - offset; - len = miter.length - ignore; -if (boffset + len > buflen) - len = buflen - boffset; -memcpy (buf + boffset, miter.addr + ignore, len); - }else { -/* Copy all of this segment (up to buflen) */ - len = miter.length; -if (boffset + len > buflen) - len = buflen - boffset; -memcpy (buf + boffset, miter.addr, len); - } - boffset += len; - } - offset += miter.length; - } - -sg_miter_stop (&miter); - -local_irq_restore (flags); -return boffset; -} -/* * allocate and map the extended descriptor */
@@ -1800,7 +1742,7 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)if (to_hash_later) {int nents =sg_count (areq->src, nbytes, &chained); -sg_copy_end_to_buffer (areq->src, nents, +sg_pcopy_to_buffer (areq->src, nents, req_ctx->bufnext, to_hash_later, nbytes - to_hash_later);
Signature · 1 line
1.8.1.4
PATCH v2 4/4 scsi_debug: fix do_device_access() with wrap around range
In reply to Akinobu Mitado_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.
This also simplifies the assignment of sdb->resid in fill_from_dev_buffer(). Because fill_from_dev_buffer() is now only called once per command execution cycle. So it is not necessary to take care to decrease sdb->resid if fill_from_dev_buffer() is called more than once.
---
* Change from v2
- Simplify the assignment of sdb->resid in fill_from_dev_buffer()
drivers/scsi/scsi_debug.c | 48 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 11 deletions(-)
drivers/scsi/scsi_debug.c +37 -11
@@ -417,10 +417,7 @@ static int fill_from_dev_buffer(struct scsi_cmnd *scp, unsigned char *arr, act_len =sg_copy_from_buffer (sdb->table.sgl, sdb->table.nents, arr, arr_len); -if (sdb->resid) - sdb->resid -= act_len; -else - sdb->resid =scsi_bufflen (scp) - act_len; + sdb->resid =scsi_bufflen (scp) - act_len;return 0 ; }
@@ -1614,24 +1611,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 +1909,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)
Signature · 1 line
1.8.1.4