This is prep work for immutable bio vecs; we first want to centralize
where bvecs are modified.

Next two patches convert some existing code to use this function.

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

diff --git a/fs/bio.c b/fs/bio.c
index 244007f..a539664 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -719,6 +719,47 @@ int bio_add_page(struct bio *bio, struct page *page, unsigned int len,
}
EXPORT_SYMBOL(bio_add_page);

+/**
+ * bio_advance - increment/complete a bio by some number of bytes
+ * @bio: bio to advance
+ * @bytes: number of bytes to complete
+ *
+ * This updates bi_sector, bi_size and bi_idx; if the number of bytes to
+ * complete doesn't align with a bvec boundary, then bv_len and bv_offset will
+ * be updated on the last bvec as well.
+ *
+ * @bio will then represent the remaining, uncompleted portion of the io.
+ */
+void bio_advance(struct bio *bio, unsigned bytes)
+{
+ if (bio_integrity(bio))
+ bio_integrity_advance(bio, bytes);
+
+ bio->bi_sector += bytes >> 0;
+ bio->bi_size -= bytes;
+
+ if (!bio->bi_size)
+ return;
+
+ while (bytes) {
+ if (unlikely(bio->bi_idx >= bio->bi_vcnt)) {
+ printk(KERN_ERR "%s: bio idx %d >= vcnt %d
",
+ __func__, bio->bi_idx, bio->bi_vcnt);
+ break;
+ }
+
+ if (bytes >= bio_iovec(bio)->bv_len) {
+ bytes -= bio_iovec(bio)->bv_len;
+ bio->bi_idx++;
+ } else {
+ bio_iovec(bio)->bv_len -= bytes;
+ bio_iovec(bio)->bv_offset += bytes;
+ bytes = 0;
+ }
+ }
+}
+EXPORT_SYMBOL(bio_advance);
+
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 7873465..6763cdf 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -248,6 +248,8 @@ extern void bio_endio(struct bio *, int);
struct request_queue;
extern int bio_phys_segments(struct request_queue *, struct bio *);

+void bio_advance(struct bio *, unsigned);
+
extern void bio_init(struct bio *);
extern void bio_reset(struct bio *);

--
1.7.12

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

This is prep work for immutable bio vecs; we first want to centralize
where bvecs are modified.

Next two patches convert some existing code to use this function.

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

diff --git a/fs/bio.c b/fs/bio.c
index 4783e31..07587c0 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -750,6 +750,47 @@ int bio_add_page(struct bio *bio, struct page *page, unsigned int len,
}
EXPORT_SYMBOL(bio_add_page);

+/**
+ * bio_advance - increment/complete a bio by some number of bytes
+ * @bio: bio to advance
+ * @bytes: number of bytes to complete
+ *
+ * This updates bi_sector, bi_size and bi_idx; if the number of bytes to
+ * complete doesn't align with a bvec boundary, then bv_len and bv_offset will
+ * be updated on the last bvec as well.
+ *
+ * @bio will then represent the remaining, uncompleted portion of the io.
+ */
+void bio_advance(struct bio *bio, unsigned bytes)
+{
+ if (bio_integrity(bio))
+ bio_integrity_advance(bio, bytes);
+
+ bio->bi_sector += bytes >> 0;
+ bio->bi_size -= bytes;
+
+ if (!bio->bi_size)
+ return;
+
+ while (bytes) {
+ if (unlikely(bio->bi_idx >= bio->bi_vcnt)) {
+ printk(KERN_ERR "%s: bio idx %d >= vcnt %d
",
+ __func__, bio->bi_idx, bio->bi_vcnt);
+ break;
+ }
+
+ if (bytes >= bio_iovec(bio)->bv_len) {
+ bytes -= bio_iovec(bio)->bv_len;
+ bio->bi_idx++;
+ } else {
+ bio_iovec(bio)->bv_len -= bytes;
+ bio_iovec(bio)->bv_offset += bytes;
+ bytes = 0;
+ }
+ }
+}
+EXPORT_SYMBOL(bio_advance);
+
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 7873465..6763cdf 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -248,6 +248,8 @@ extern void bio_endio(struct bio *, int);
struct request_queue;
extern int bio_phys_segments(struct request_queue *, struct bio *);

+void bio_advance(struct bio *, unsigned);
+
extern void bio_init(struct bio *);
extern void bio_reset(struct bio *);

--
1.7.12

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

On Mon, Sep 10, 2012 at 05:22:13PM -0700, Kent Overstreet wrote:
> +/**
> + * bio_advance - increment/complete a bio by some number of bytes
> + * @bio: bio to advance
> + * @bytes: number of bytes to complete
> + *
> + * This updates bi_sector, bi_size and bi_idx; if the number of bytes to
> + * complete doesn't align with a bvec boundary, then bv_len and bv_offset will
> + * be updated on the last bvec as well.
> + *
> + * @bio will then represent the remaining, uncompleted portion of the io.
> + */
> +void bio_advance(struct bio *bio, unsigned bytes)
> +{
> + if (bio_integrity(bio))
> + bio_integrity_advance(bio, bytes);
> +
> + bio->bi_sector += bytes >> 0;

Hmmm.... bytes >> 0?

> + bio->bi_size -= bytes;
> +
> + if (!bio->bi_size)
> + return;
> +
> + while (bytes) {
> + if (unlikely(bio->bi_idx >= bio->bi_vcnt)) {
> + printk(KERN_ERR "%s: bio idx %d >= vcnt %d
",

pr_err() is preferred but maybe WARN_ON_ONCE() is better fit here?
This happening would be a bug, right?

Thanks.

--
tejun

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

On Thu, Sep 20, 2012 at 02:58:27PM -0700, Tejun Heo wrote:
> On Mon, Sep 10, 2012 at 05:22:13PM -0700, Kent Overstreet wrote:
> > +/**
> > + * bio_advance - increment/complete a bio by some number of bytes
> > + * @bio: bio to advance
> > + * @bytes: number of bytes to complete
> > + *
> > + * This updates bi_sector, bi_size and bi_idx; if the number of bytes to
> > + * complete doesn't align with a bvec boundary, then bv_len and bv_offset will
> > + * be updated on the last bvec as well.
> > + *
> > + * @bio will then represent the remaining, uncompleted portion of the io.
> > + */
> > +void bio_advance(struct bio *bio, unsigned bytes)
> > +{
> > + if (bio_integrity(bio))
> > + bio_integrity_advance(bio, bytes);
> > +
> > + bio->bi_sector += bytes >> 0;
>
> Hmmm.... bytes >> 0?

Whoops...

> > + bio->bi_size -= bytes;
> > +
> > + if (!bio->bi_size)
> > + return;
> > +
> > + while (bytes) {
> > + if (unlikely(bio->bi_idx >= bio->bi_vcnt)) {
> > + printk(KERN_ERR "%s: bio idx %d >= vcnt %d
",
>
> pr_err() is preferred but maybe WARN_ON_ONCE() is better fit here?
> This happening would be a bug, right?

I just cut and pasted that from blk_update_request(), which is what the
next patch refactors...

But yes it would be a bug. It gets converted to a BUG_ON() in a later
patch (not in this series), as this gets further abstracted into a
wrapper around bvec_advance_iter() which doesn't know about struct bio
(as bio integrity gets its own iterator).

Might drop it entirely, depending on what exactly I end up doing with
bi_vcnt...

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

Hello,

On Thu, Sep 20, 2012 at 04:13:08PM -0700, Kent Overstreet wrote:
> I just cut and pasted that from blk_update_request(), which is what the
> next patch refactors...

Yeah, well, that was written when we didn't have WARNs.

> But yes it would be a bug. It gets converted to a BUG_ON() in a later
> patch (not in this series), as this gets further abstracted into a
> wrapper around bvec_advance_iter() which doesn't know about struct bio
> (as bio integrity gets its own iterator).

WARN() generally preferable unless there's no way at all to continue.
Storage layer could be a bit different if immediate danger for data
corruption exists but the general consensus seems that we're too
trigger happy with BUG_ON()s.

Thanks.

--
tejun

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

On Thu, Sep 20, 2012 at 04:25:06PM -0700, Tejun Heo wrote:
> Hello,
>
> On Thu, Sep 20, 2012 at 04:13:08PM -0700, Kent Overstreet wrote:
> > I just cut and pasted that from blk_update_request(), which is what the
> > next patch refactors...
>
> Yeah, well, that was written when we didn't have WARNs.
>
> > But yes it would be a bug. It gets converted to a BUG_ON() in a later
> > patch (not in this series), as this gets further abstracted into a
> > wrapper around bvec_advance_iter() which doesn't know about struct bio
> > (as bio integrity gets its own iterator).
>
> WARN() generally preferable unless there's no way at all to continue.
> Storage layer could be a bit different if immediate danger for data
> corruption exists but the general consensus seems that we're too
> trigger happy with BUG_ON()s.

Yeah. Changed it to a WARN_ONCE().

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

This is prep work for immutable bio vecs; we first want to centralize
where bvecs are modified.

Next two patches convert some existing code to use this function.

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

diff --git a/fs/bio.c b/fs/bio.c
index ac8b443..9a3935f 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -750,6 +750,47 @@ int bio_add_page(struct bio *bio, struct page *page, unsigned int len,
}
EXPORT_SYMBOL(bio_add_page);

+/**
+ * bio_advance - increment/complete a bio by some number of bytes
+ * @bio: bio to advance
+ * @bytes: number of bytes to complete
+ *
+ * This updates bi_sector, bi_size and bi_idx; if the number of bytes to
+ * complete doesn't align with a bvec boundary, then bv_len and bv_offset will
+ * be updated on the last bvec as well.
+ *
+ * @bio will then represent the remaining, uncompleted portion of the io.
+ */
+void bio_advance(struct bio *bio, unsigned bytes)
+{
+ if (bio_integrity(bio))
+ bio_integrity_advance(bio, bytes);
+
+ bio->bi_sector += bytes >> 9;
+ bio->bi_size -= bytes;
+
+ if (!bio->bi_size)
+ return;
+
+ while (bytes) {
+ if (unlikely(bio->bi_idx >= bio->bi_vcnt)) {
+ WARN_ONCE(1, "bio idx %d >= vcnt %d
",
+ bio->bi_idx, bio->bi_vcnt);
+ break;
+ }
+
+ if (bytes >= bio_iovec(bio)->bv_len) {
+ bytes -= bio_iovec(bio)->bv_len;
+ bio->bi_idx++;
+ } else {
+ bio_iovec(bio)->bv_len -= bytes;
+ bio_iovec(bio)->bv_offset += bytes;
+ bytes = 0;
+ }
+ }
+}
+EXPORT_SYMBOL(bio_advance);
+
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 a7c021d..4e32be1 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -251,6 +251,8 @@ extern void bio_endio(struct bio *, int);
struct request_queue;
extern int bio_phys_segments(struct request_queue *, struct bio *);

+extern void bio_advance(struct bio *, unsigned);
+
extern void bio_init(struct bio *);
extern void bio_reset(struct bio *);

--
1.7.12

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

This is prep work for immutable bio vecs; we first want to centralize
where bvecs are modified.

Next two patches convert some existing code to use this function.

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

diff --git a/fs/bio.c b/fs/bio.c
index f16aa6b..dc3452e 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -750,6 +750,47 @@ int bio_add_page(struct bio *bio, struct page *page, unsigned int len,
}
EXPORT_SYMBOL(bio_add_page);

+/**
+ * bio_advance - increment/complete a bio by some number of bytes
+ * @bio: bio to advance
+ * @bytes: number of bytes to complete
+ *
+ * This updates bi_sector, bi_size and bi_idx; if the number of bytes to
+ * complete doesn't align with a bvec boundary, then bv_len and bv_offset will
+ * be updated on the last bvec as well.
+ *
+ * @bio will then represent the remaining, uncompleted portion of the io.
+ */
+void bio_advance(struct bio *bio, unsigned bytes)
+{
+ if (bio_integrity(bio))
+ bio_integrity_advance(bio, bytes);
+
+ bio->bi_sector += bytes >> 9;
+ bio->bi_size -= bytes;
+
+ if (bio->bi_rw & BIO_NO_ADVANCE_ITER_MASK)
+ return;
+
+ while (bytes) {
+ if (unlikely(bio->bi_idx >= bio->bi_vcnt)) {
+ WARN_ONCE(1, "bio idx %d >= vcnt %d
",
+ bio->bi_idx, bio->bi_vcnt);
+ break;
+ }
+
+ if (bytes >= bio_iovec(bio)->bv_len) {
+ bytes -= bio_iovec(bio)->bv_len;
+ bio->bi_idx++;
+ } else {
+ bio_iovec(bio)->bv_len -= bytes;
+ bio_iovec(bio)->bv_offset += bytes;
+ bytes = 0;
+ }
+ }
+}
+EXPORT_SYMBOL(bio_advance);
+
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 669b1cb..fcb4dba 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -248,6 +248,8 @@ extern void bio_endio(struct bio *, int);
struct request_queue;
extern int bio_phys_segments(struct request_queue *, struct bio *);

+extern void bio_advance(struct bio *, unsigned);
+
extern void bio_init(struct bio *);
extern void bio_reset(struct bio *);

diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index cdf1119..c178d25 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -197,6 +197,8 @@ enum rq_flag_bits {
REQ_SECURE)
#define REQ_CLONE_MASK REQ_COMMON_MASK

+#define BIO_NO_ADVANCE_ITER_MASK (REQ_DISCARD|REQ_WRITE_SAME)
+
/* This mask is used for both bio and request merge checking */
#define REQ_NOMERGE_FLAGS
(REQ_NOMERGE | REQ_STARTED | REQ_SOFTBARRIER | REQ_FLUSH | REQ_FUA)
--
1.7.12

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

This is prep work for immutable bio vecs; we first want to centralize
where bvecs are modified.

Next two patches convert some existing code to use this function.

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

diff --git a/fs/bio.c b/fs/bio.c
index 4783e31..07587c0 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -750,6 +750,47 @@ int bio_add_page(struct bio *bio, struct page *page, unsigned int len,
}
EXPORT_SYMBOL(bio_add_page);

+/**
+ * bio_advance - increment/complete a bio by some number of bytes
+ * @bio: bio to advance
+ * @bytes: number of bytes to complete
+ *
+ * This updates bi_sector, bi_size and bi_idx; if the number of bytes to
+ * complete doesn't align with a bvec boundary, then bv_len and bv_offset will
+ * be updated on the last bvec as well.
+ *
+ * @bio will then represent the remaining, uncompleted portion of the io.
+ */
+void bio_advance(struct bio *bio, unsigned bytes)
+{
+ if (bio_integrity(bio))
+ bio_integrity_advance(bio, bytes);
+
+ bio->bi_sector += bytes >> 0;
+ bio->bi_size -= bytes;
+
+ if (!bio->bi_size)
+ return;
+
+ while (bytes) {
+ if (unlikely(bio->bi_idx >= bio->bi_vcnt)) {
+ printk(KERN_ERR "%s: bio idx %d >= vcnt %d
",
+ __func__, bio->bi_idx, bio->bi_vcnt);
+ break;
+ }
+
+ if (bytes >= bio_iovec(bio)->bv_len) {
+ bytes -= bio_iovec(bio)->bv_len;
+ bio->bi_idx++;
+ } else {
+ bio_iovec(bio)->bv_len -= bytes;
+ bio_iovec(bio)->bv_offset += bytes;
+ bytes = 0;
+ }
+ }
+}
+EXPORT_SYMBOL(bio_advance);
+
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 7873465..6763cdf 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -248,6 +248,8 @@ extern void bio_endio(struct bio *, int);
struct request_queue;
extern int bio_phys_segments(struct request_queue *, struct bio *);

+void bio_advance(struct bio *, unsigned);
+
extern void bio_init(struct bio *);
extern void bio_reset(struct bio *);

--
1.7.12

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