Mailing List Archive

[PATCH] loop 2/9 absorb bio_copy
bio_copy is used only by the loop driver, which already has to walk the
bio segments itself: so it makes sense to change it from bio.c export
to loop.c static, as prelude to working upon it there.
bio_copy itself is unchanged by this patch, with one exception. On oom
failure it must use bio_put, instead of mempool_free to static bio_pool:
which it should have been doing all along - it was leaking the veclist.
--- loop1/drivers/block/loop.c Mon Jun 9 10:29:01 2003
+++ loop2/drivers/block/loop.c Mon Jun 9 10:32:06 2003
@@ -439,6 +439,74 @@
return 0;
}

+static struct bio *bio_copy(struct bio *bio, int gfp_mask, int copy)
+{
+ struct bio *b = bio_alloc(gfp_mask, bio->bi_vcnt);
+ unsigned long flags = 0; /* gcc silly */
+ struct bio_vec *bv;
+ int i;
+
+ if (unlikely(!b))
+ return NULL;
+
+ /*
+ * iterate iovec list and alloc pages + copy data
+ */
+ __bio_for_each_segment(bv, bio, i, 0) {
+ struct bio_vec *bbv = &b->bi_io_vec[i];
+ char *vfrom, *vto;
+
+ bbv->bv_page = alloc_page(gfp_mask);
+ if (bbv->bv_page == NULL)
+ goto oom;
+
+ bbv->bv_len = bv->bv_len;
+ bbv->bv_offset = bv->bv_offset;
+
+ /*
+ * if doing a copy for a READ request, no need
+ * to memcpy page data
+ */
+ if (!copy)
+ continue;
+
+ if (gfp_mask & __GFP_WAIT) {
+ vfrom = kmap(bv->bv_page);
+ vto = kmap(bbv->bv_page);
+ } else {
+ local_irq_save(flags);
+ vfrom = kmap_atomic(bv->bv_page, KM_BIO_SRC_IRQ);
+ vto = kmap_atomic(bbv->bv_page, KM_BIO_DST_IRQ);
+ }
+
+ memcpy(vto + bbv->bv_offset, vfrom + bv->bv_offset, bv->bv_len);
+ if (gfp_mask & __GFP_WAIT) {
+ kunmap(bbv->bv_page);
+ kunmap(bv->bv_page);
+ } else {
+ kunmap_atomic(vto, KM_BIO_DST_IRQ);
+ kunmap_atomic(vfrom, KM_BIO_SRC_IRQ);
+ local_irq_restore(flags);
+ }
+ }
+
+ b->bi_sector = bio->bi_sector;
+ b->bi_bdev = bio->bi_bdev;
+ b->bi_rw = bio->bi_rw;
+
+ b->bi_vcnt = bio->bi_vcnt;
+ b->bi_size = bio->bi_size;
+
+ return b;
+
+oom:
+ while (--i >= 0)
+ __free_page(b->bi_io_vec[i].bv_page);
+
+ bio_put(bio);
+ return NULL;
+}
+
static struct bio *loop_get_buffer(struct loop_device *lo, struct bio *rbh)
{
struct bio *bio;
--- loop1/fs/bio.c Mon Jun 9 10:14:59 2003
+++ loop2/fs/bio.c Mon Jun 9 10:32:06 2003
@@ -261,84 +261,6 @@
}

/**
- * bio_copy - create copy of a bio
- * @bio: bio to copy
- * @gfp_mask: allocation priority
- * @copy: copy data to allocated bio
- *
- * Create a copy of a &bio. Caller will own the returned bio and
- * the actual data it points to. Reference count of returned
- * bio will be one.
- */
-struct bio *bio_copy(struct bio *bio, int gfp_mask, int copy)
-{
- struct bio *b = bio_alloc(gfp_mask, bio->bi_vcnt);
- unsigned long flags = 0; /* gcc silly */
- struct bio_vec *bv;
- int i;
-
- if (unlikely(!b))
- return NULL;
-
- /*
- * iterate iovec list and alloc pages + copy data
- */
- __bio_for_each_segment(bv, bio, i, 0) {
- struct bio_vec *bbv = &b->bi_io_vec[i];
- char *vfrom, *vto;
-
- bbv->bv_page = alloc_page(gfp_mask);
- if (bbv->bv_page == NULL)
- goto oom;
-
- bbv->bv_len = bv->bv_len;
- bbv->bv_offset = bv->bv_offset;
-
- /*
- * if doing a copy for a READ request, no need
- * to memcpy page data
- */
- if (!copy)
- continue;
-
- if (gfp_mask & __GFP_WAIT) {
- vfrom = kmap(bv->bv_page);
- vto = kmap(bbv->bv_page);
- } else {
- local_irq_save(flags);
- vfrom = kmap_atomic(bv->bv_page, KM_BIO_SRC_IRQ);
- vto = kmap_atomic(bbv->bv_page, KM_BIO_DST_IRQ);
- }
-
- memcpy(vto + bbv->bv_offset, vfrom + bv->bv_offset, bv->bv_len);
- if (gfp_mask & __GFP_WAIT) {
- kunmap(bbv->bv_page);
- kunmap(bv->bv_page);
- } else {
- kunmap_atomic(vto, KM_BIO_DST_IRQ);
- kunmap_atomic(vfrom, KM_BIO_SRC_IRQ);
- local_irq_restore(flags);
- }
- }
-
- b->bi_sector = bio->bi_sector;
- b->bi_bdev = bio->bi_bdev;
- b->bi_rw = bio->bi_rw;
-
- b->bi_vcnt = bio->bi_vcnt;
- b->bi_size = bio->bi_size;
-
- return b;
-
-oom:
- while (--i >= 0)
- __free_page(b->bi_io_vec[i].bv_page);
-
- mempool_free(b, bio_pool);
- return NULL;
-}
-
-/**
* bio_get_nr_vecs - return approx number of vecs
* @bdev: I/O target
*
@@ -907,7 +829,6 @@
EXPORT_SYMBOL(bio_put);
EXPORT_SYMBOL(bio_endio);
EXPORT_SYMBOL(bio_init);
-EXPORT_SYMBOL(bio_copy);
EXPORT_SYMBOL(__bio_clone);
EXPORT_SYMBOL(bio_clone);
EXPORT_SYMBOL(bio_phys_segments);
--- loop1/include/linux/bio.h Mon Jun 9 10:15:00 2003
+++ loop2/include/linux/bio.h Mon Jun 9 10:32:06 2003
@@ -235,7 +235,6 @@

extern inline void __bio_clone(struct bio *, struct bio *);
extern struct bio *bio_clone(struct bio *, int);
-extern struct bio *bio_copy(struct bio *, int, int);

extern inline void bio_init(struct bio *);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] loop 2/9 absorb bio_copy [ In reply to ]
On Tue, Jun 10 2003, Hugh Dickins wrote:
> bio_copy is used only by the loop driver, which already has to walk the
> bio segments itself: so it makes sense to change it from bio.c export
> to loop.c static, as prelude to working upon it there.
>
> bio_copy itself is unchanged by this patch, with one exception. On oom
> failure it must use bio_put, instead of mempool_free to static bio_pool:
> which it should have been doing all along - it was leaking the veclist.
I don't think this is is a particularly good idea, it's pretty core bio
functionality that should be left alone in bio.c imho.
Is there a real reason you want to do this apart from 'loop is the only
(current) user'?
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] loop 2/9 absorb bio_copy [ In reply to ]
On Tue, Jun 10 2003, Hugh Dickins wrote:
> On Tue, 10 Jun 2003, Jens Axboe wrote:
> > On Tue, Jun 10 2003, Hugh Dickins wrote:
> > > bio_copy is used only by the loop driver, which already has to walk the
> > > bio segments itself: so it makes sense to change it from bio.c export
> > > to loop.c static, as prelude to working upon it there.
> >
> > I don't think this is is a particularly good idea, it's pretty core bio
> > functionality that should be left alone in bio.c imho.
> >
> > Is there a real reason you want to do this apart from 'loop is the only
> > (current) user'?
>
> As I said, loop already has to walk the bio segments itself elsewhere,
> and a lot of what bio_copy does (e.g. copying data) it doesn't need done,
> and other things it does (same gfp_mask for two very different allocations)
> don't suit loop very well. By all means add bio_copy back into fs/bio.c
> when something else needs that functionality?
Alright, I guess I can live with that as there's no direct need for it
elsewhere right now. Just doesn't feel right.
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] loop 2/9 absorb bio_copy [ In reply to ]
On Tue, 10 Jun 2003, Jens Axboe wrote:
> On Tue, Jun 10 2003, Hugh Dickins wrote:
> > bio_copy is used only by the loop driver, which already has to walk the
> > bio segments itself: so it makes sense to change it from bio.c export
> > to loop.c static, as prelude to working upon it there.
>
> I don't think this is is a particularly good idea, it's pretty core bio
> functionality that should be left alone in bio.c imho.
>
> Is there a real reason you want to do this apart from 'loop is the only
> (current) user'?
As I said, loop already has to walk the bio segments itself elsewhere,
and a lot of what bio_copy does (e.g. copying data) it doesn't need done,
and other things it does (same gfp_mask for two very different allocations)
don't suit loop very well. By all means add bio_copy back into fs/bio.c
when something else needs that functionality?
Hugh
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/