| Message ID | 1347322957-25260-23-git-send-email-koverstreet@google.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
On Mon, Sep 10, 2012 at 05:22:33PM -0700, Kent Overstreet wrote: > + bio_for_each_segment_all(bv, bio, i) { > + bv->bv_page = alloc_page(gfp_mask); > + if (!bv->bv_page) { > + while (bv-- != bio->bi_io_vec) > + __free_page(bv->bv_page); I don't know. I feel stupid. I think it's because the loop variable changes between loop condition test and actual body of loop. How about the following? It is pointing to the member of the same array so I think it's not even violating pointer comparison rules. while (--bv >= bio->bi_io_vec) __free_page(bv->bv_page);
On Thu, Sep 20, 2012 at 05:47:11PM -0700, Tejun Heo wrote: > On Mon, Sep 10, 2012 at 05:22:33PM -0700, Kent Overstreet wrote: > > + bio_for_each_segment_all(bv, bio, i) { > > + bv->bv_page = alloc_page(gfp_mask); > > + if (!bv->bv_page) { > > + while (bv-- != bio->bi_io_vec) > > + __free_page(bv->bv_page); > > I don't know. I feel stupid. I think it's because the loop variable > changes between loop condition test and actual body of loop. How > about the following? It is pointing to the member of the same array > so I think it's not even violating pointer comparison rules. > > while (--bv >= bio->bi_io_vec) > __free_page(bv->bv_page); I can't remember why I did it that way, but I think I like yours better - I'll change it. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/fs/bio.c b/fs/bio.c index d88ad77..65e6eac 100644 --- a/fs/bio.c +++ b/fs/bio.c @@ -828,6 +828,34 @@ void bio_advance(struct bio *bio, unsigned bytes) EXPORT_SYMBOL(bio_advance); /** + * bio_alloc_pages - allocates a single page for each bvec in a bio + * @bio: bio to allocate pages for + * @gfp_mask: flags for allocation + * + * Allocates pages up to @bio->bi_vcnt. + * + * Returns 0 on success, -ENOMEM on failure. On failure, any allocated pages are + * freed. + */ +int bio_alloc_pages(struct bio *bio, gfp_t gfp_mask) +{ + int i; + struct bio_vec *bv; + + bio_for_each_segment_all(bv, bio, i) { + bv->bv_page = alloc_page(gfp_mask); + if (!bv->bv_page) { + while (bv-- != bio->bi_io_vec) + __free_page(bv->bv_page); + return -ENOMEM; + } + } + + return 0; +} +EXPORT_SYMBOL(bio_alloc_pages); + +/** * bio_copy_data - copy contents of data buffers from one chain of bios to * another * @src: source bio list diff --git a/include/linux/bio.h b/include/linux/bio.h index b433ff8..bd45154 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -289,6 +289,7 @@ static inline void bio_flush_dcache_pages(struct bio *bi) #endif extern void bio_copy_data(struct bio *dst, struct bio *src); +extern int bio_alloc_pages(struct bio *bio, gfp_t gfp); extern struct bio *bio_copy_user(struct request_queue *, struct rq_map_data *, unsigned long, unsigned int, int, gfp_t);
More utility code to replace stuff that's getting open coded. Signed-off-by: Kent Overstreet <koverstreet@google.com> CC: Jens Axboe <axboe@kernel.dk> --- fs/bio.c | 28 ++++++++++++++++++++++++++++ include/linux/bio.h | 1 + 2 files changed, 29 insertions(+)