Thread (7 messages) 7 messages, 3 authors, 2013-06-08

Re: [PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

From: Akinobu Mita <akinobu.mita@gmail.com>
Date: 2013-06-08 14:28:08
Also in: linux-scsi, lkml

2013/6/7 Tejun Heo [off-list ref]:
Hello,

On Thu, Jun 06, 2013 at 09:52:56PM +0900, Akinobu Mita wrote:
quoted
+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.
quoted
+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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help