This gets open coded quite a bit and it's tricky to get right, so make a
generic version and convert some existing users over to it instead.

Signed-off-by: Kent Overstreet <koverstreet@google.com>
CC: Jens Axboe <axboe@kernel.dk>
---
fs/bio.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ +++
include/linux/bio.h | 2 ++
2 files changed, 72 insertions(+)

diff --git a/fs/bio.c b/fs/bio.c
index 1342a16..7fb9f4e 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -827,6 +827,76 @@ void bio_advance(struct bio *bio, unsigned bytes)
}
EXPORT_SYMBOL(bio_advance);

+/**
+ * bio_copy_data - copy contents of data buffers from one chain of bios to
+ * another
+ * @src: source bio list
+ * @dst: destination bio list
+ *
+ * If @src and @dst are single bios, bi_next must be NULL - otherwise, treats
+ * @src and @dst as linked lists of bios.
+ *
+ * Stops when it reaches the end of either @src or @dst - that is, copies
+ * min(src->bi_size, dst->bi_size) bytes (or the equivalent for lists of bios).
+ */
+void bio_copy_data(struct bio *dst, struct bio *src)
+{
+ struct bio_vec *src_bv, *dst_bv;
+ unsigned src_offset, dst_offset, bytes;
+ void *src_p, *dst_p;
+
+ src_bv = bio_iovec(src);
+ dst_bv = bio_iovec(dst);
+
+ src_offset = src_bv->bv_offset;
+ dst_offset = dst_bv->bv_offset;
+
+ while (1) {
+ if (src_offset == src_bv->bv_offset + src_bv->bv_len) {
+ src_bv++;
+ if (src_bv == bio_iovec_idx(src, src->bi_vcnt)) {
+ src = src->bi_next;
+ if (!src)
+ break;
+
+ src_bv = bio_iovec(src);
+ }
+
+ src_offset = src_bv->bv_offset;
+ }
+
+ if (dst_offset == dst_bv->bv_offset + dst_bv->bv_len) {
+ dst_bv++;
+ if (dst_bv == bio_iovec_idx(dst, dst->bi_vcnt)) {
+ dst = dst->bi_next;
+ if (!dst)
+ break;
+
+ dst_bv = bio_iovec(dst);
+ }
+
+ dst_offset = dst_bv->bv_offset;
+ }
+
+ bytes = min(dst_bv->bv_offset + dst_bv->bv_len - dst_offset,
+ src_bv->bv_offset + src_bv->bv_len - src_offset);
+
+ src_p = kmap_atomic(src_bv->bv_page);
+ dst_p = kmap_atomic(dst_bv->bv_page);
+
+ memcpy(dst_p + dst_bv->bv_offset,
+ src_p + src_bv->bv_offset,
+ bytes);
+
+ kunmap_atomic(dst_p);
+ kunmap_atomic(src_p);
+
+ src_offset += bytes;
+ dst_offset += bytes;
+ }
+}
+EXPORT_SYMBOL(bio_copy_data);
+
struct bio_map_data {
struct bio_vec *iovecs;
struct sg_iovec *sgvecs;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 949c48a..92015ce 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -286,6 +286,8 @@ static inline void bio_flush_dcache_pages(struct bio *bi)
}
#endif

+extern void bio_copy_data(struct bio *dst, struct bio *src);
+
extern struct bio *bio_copy_user(struct request_queue *, struct rq_map_data *,
unsigned long, unsigned int, int, gfp_t);
extern struct bio *bio_copy_user_iov(struct request_queue *,
--
1.7.12

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Hello,

On Mon, Sep 10, 2012 at 05:22:26PM -0700, Kent Overstreet wrote:
> +void bio_copy_data(struct bio *dst, struct bio *src)
> +{
...
> + src_p = kmap_atomic(src_bv->bv_page);
> + dst_p = kmap_atomic(dst_bv->bv_page);
> +
> + memcpy(dst_p + dst_bv->bv_offset,
> + src_p + src_bv->bv_offset,
> + bytes);
> +
> + kunmap_atomic(dst_p);
> + kunmap_atomic(src_p);

Wrap these in preempt_disable/enable() to allow the function to be
called from any context?

Thanks.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Thu, Sep 20, 2012 at 05:06:32PM -0700, Tejun Heo wrote:
> Hello,
>
> On Mon, Sep 10, 2012 at 05:22:26PM -0700, Kent Overstreet wrote:
> > +void bio_copy_data(struct bio *dst, struct bio *src)
> > +{
> ...
> > + src_p = kmap_atomic(src_bv->bv_page);
> > + dst_p = kmap_atomic(dst_bv->bv_page);
> > +
> > + memcpy(dst_p + dst_bv->bv_offset,
> > + src_p + src_bv->bv_offset,
> > + bytes);
> > +
> > + kunmap_atomic(dst_p);
> > + kunmap_atomic(src_p);
>
> Wrap these in preempt_disable/enable() to allow the function to be
> called from any context?

I checked the implementation of kmap_atomic(), it already does
preempt_disable() so it's safe in process context - if I understand
correctly it needs local_irq_save()/restore() to be safe in any context
and I figured calling this from irq context is not the norm so that
should be the caller's responsibility.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Thu, Sep 20, 2012 at 05:06:32PM -0700, Tejun Heo wrote:
> Hello,
>
> On Mon, Sep 10, 2012 at 05:22:26PM -0700, Kent Overstreet wrote:
> > +void bio_copy_data(struct bio *dst, struct bio *src)
> > +{
> ...
> > + src_p = kmap_atomic(src_bv->bv_page);
> > + dst_p = kmap_atomic(dst_bv->bv_page);
> > +
> > + memcpy(dst_p + dst_bv->bv_offset,
> > + src_p + src_bv->bv_offset,
> > + bytes);
> > +
> > + kunmap_atomic(dst_p);
> > + kunmap_atomic(src_p);
>
> Wrap these in preempt_disable/enable() to allow the function to be
> called from any context?

Ooh, and maybe return the amount of copied data?

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Thu, Sep 20, 2012 at 05:09:47PM -0700, Tejun Heo wrote:
> On Thu, Sep 20, 2012 at 05:06:32PM -0700, Tejun Heo wrote:
> > Hello,
> >
> > On Mon, Sep 10, 2012 at 05:22:26PM -0700, Kent Overstreet wrote:
> > > +void bio_copy_data(struct bio *dst, struct bio *src)
> > > +{
> > ...
> > > + src_p = kmap_atomic(src_bv->bv_page);
> > > + dst_p = kmap_atomic(dst_bv->bv_page);
> > > +
> > > + memcpy(dst_p + dst_bv->bv_offset,
> > > + src_p + src_bv->bv_offset,
> > > + bytes);
> > > +
> > > + kunmap_atomic(dst_p);
> > > + kunmap_atomic(src_p);
> >
> > Wrap these in preempt_disable/enable() to allow the function to be
> > called from any context?
>
> Ooh, and maybe return the amount of copied data?

Possibly, but I think I want to wait until a user needs it before adding
something like that.

>From looking at other code that copies bio data, a parameter that
specifies the amount of data to be copied might be more useful.

I'm not sure I've seen all the places where bio data is copied yet, so
I've just been waiting until I find more uses to make it do more.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

On Thu, Sep 20, 2012 at 05:09:45PM -0700, Kent Overstreet wrote:
> On Thu, Sep 20, 2012 at 05:06:32PM -0700, Tejun Heo wrote:
> > Hello,
> >
> > On Mon, Sep 10, 2012 at 05:22:26PM -0700, Kent Overstreet wrote:
> > > +void bio_copy_data(struct bio *dst, struct bio *src)
> > > +{
> > ...
> > > + src_p = kmap_atomic(src_bv->bv_page);
> > > + dst_p = kmap_atomic(dst_bv->bv_page);
> > > +
> > > + memcpy(dst_p + dst_bv->bv_offset,
> > > + src_p + src_bv->bv_offset,
> > > + bytes);
> > > +
> > > + kunmap_atomic(dst_p);
> > > + kunmap_atomic(src_p);
> >
> > Wrap these in preempt_disable/enable() to allow the function to be
> > called from any context?
>
> I checked the implementation of kmap_atomic(), it already does
> preempt_disable() so it's safe in process context - if I understand
> correctly it needs local_irq_save()/restore() to be safe in any context
> and I figured calling this from irq context is not the norm so that
> should be the caller's responsibility.

Ooh, that means the patch I just sent Andrew about sg_mapping_iter is
still too strict.

Thanks.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

This gets open coded quite a bit and it's tricky to get right, so make a
generic version and convert some existing users over to it instead.

Signed-off-by: Kent Overstreet <koverstreet@google.com>
CC: Jens Axboe <axboe@kernel.dk>
---
fs/bio.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ +++
include/linux/bio.h | 2 ++
2 files changed, 72 insertions(+)

diff --git a/fs/bio.c b/fs/bio.c
index 9a30dcf..d433825 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -827,6 +827,76 @@ void bio_advance(struct bio *bio, unsigned bytes)
}
EXPORT_SYMBOL(bio_advance);

+/**
+ * bio_copy_data - copy contents of data buffers from one chain of bios to
+ * another
+ * @src: source bio list
+ * @dst: destination bio list
+ *
+ * If @src and @dst are single bios, bi_next must be NULL - otherwise, treats
+ * @src and @dst as linked lists of bios.
+ *
+ * Stops when it reaches the end of either @src or @dst - that is, copies
+ * min(src->bi_size, dst->bi_size) bytes (or the equivalent for lists of bios).
+ */
+void bio_copy_data(struct bio *dst, struct bio *src)
+{
+ struct bio_vec *src_bv, *dst_bv;
+ unsigned src_offset, dst_offset, bytes;
+ void *src_p, *dst_p;
+
+ src_bv = bio_iovec(src);
+ dst_bv = bio_iovec(dst);
+
+ src_offset = src_bv->bv_offset;
+ dst_offset = dst_bv->bv_offset;
+
+ while (1) {
+ if (src_offset == src_bv->bv_offset + src_bv->bv_len) {
+ src_bv++;
+ if (src_bv == bio_iovec_idx(src, src->bi_vcnt)) {
+ src = src->bi_next;
+ if (!src)
+ break;
+
+ src_bv = bio_iovec(src);
+ }
+
+ src_offset = src_bv->bv_offset;
+ }
+
+ if (dst_offset == dst_bv->bv_offset + dst_bv->bv_len) {
+ dst_bv++;
+ if (dst_bv == bio_iovec_idx(dst, dst->bi_vcnt)) {
+ dst = dst->bi_next;
+ if (!dst)
+ break;
+
+ dst_bv = bio_iovec(dst);
+ }
+
+ dst_offset = dst_bv->bv_offset;
+ }
+
+ bytes = min(dst_bv->bv_offset + dst_bv->bv_len - dst_offset,
+ src_bv->bv_offset + src_bv->bv_len - src_offset);
+
+ src_p = kmap_atomic(src_bv->bv_page);
+ dst_p = kmap_atomic(dst_bv->bv_page);
+
+ memcpy(dst_p + dst_bv->bv_offset,
+ src_p + src_bv->bv_offset,
+ bytes);
+
+ kunmap_atomic(dst_p);
+ kunmap_atomic(src_p);
+
+ src_offset += bytes;
+ dst_offset += bytes;
+ }
+}
+EXPORT_SYMBOL(bio_copy_data);
+
struct bio_map_data {
struct bio_vec *iovecs;
struct sg_iovec *sgvecs;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 3bf696b..467e101 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -289,6 +289,8 @@ static inline void bio_flush_dcache_pages(struct bio *bi)
}
#endif

+extern void bio_copy_data(struct bio *dst, struct bio *src);
+
extern struct bio *bio_copy_user(struct request_queue *, struct rq_map_data *,
unsigned long, unsigned int, int, gfp_t);
extern struct bio *bio_copy_user_iov(struct request_queue *,
--
1.7.12

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel