* [PATCH] loop 1/9 file use highmem
@ 2003-06-10 15:30 Hugh Dickins
2003-06-10 15:31 ` [PATCH] loop 2/9 absorb bio_copy Hugh Dickins
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:30 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
Here's the first of nine small patches to the loop driver,
mostly to reduce its pressure on zone normal when there's highmem.
I don't pretend these fix every loop hang I see and you not,
nor have I completed reinvestigating those against latest kernel.
These patches just remove what I hope we'll agree is silliness,
setting a base for further work.
(Some could conceivably make some loads worse: making better use of
highmem for data could in theory expose kmap deadlocks, I've never seen,
or permit so much more data in transit that mempool exhaustion wins:
if so, we fix those issues, rather than depending on mistakes.)
Based on 2.5.70-mm7. Aggregate diffstat is:
drivers/block/loop.c | 71 +++++++++++++++++++++++++++------------------
fs/bio.c | 79 ---------------------------------------------------
include/linux/bio.h | 1
include/linux/loop.h | 1
4 files changed, 43 insertions(+), 109 deletions(-)
loop 1/9 file use highmem
When loop restricts underlying file's allocation mask to avoid
deadlock, it unintentionally masks out its highmem capability,
making failures at the underlying level much more likely.
--- 2.5.70-mm7/drivers/block/loop.c Mon Jun 9 10:14:55 2003
+++ loop1/drivers/block/loop.c Mon Jun 9 10:29:01 2003
@@ -714,7 +714,7 @@
goto out_putf;
}
lo->old_gfp_mask = inode->i_mapping->gfp_mask;
- inode->i_mapping->gfp_mask = GFP_NOIO;
+ inode->i_mapping->gfp_mask &= ~(__GFP_IO|__GFP_FS);
set_blocksize(bdev, lo_blocksize);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 2/9 absorb bio_copy
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
@ 2003-06-10 15:31 ` Hugh Dickins
2003-06-10 15:37 ` Jens Axboe
2003-06-10 15:31 ` [PATCH] loop 3/9 loop bio renaming Hugh Dickins
` (6 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
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 *);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 3/9 loop bio renaming
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
2003-06-10 15:31 ` [PATCH] loop 2/9 absorb bio_copy Hugh Dickins
@ 2003-06-10 15:31 ` Hugh Dickins
2003-06-10 15:32 ` [PATCH] loop 4/9 copy bio not data Hugh Dickins
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
Now it's in loop not bio, better rename bio_copy to loop_copy_bio: loop
prefers names that way; and bio_transfer better named loop_transfer_bio.
Rename bio,b to rbh,bio to follow call from loop_get_buffer more easily.
--- loop2/drivers/block/loop.c Mon Jun 9 10:32:06 2003
+++ loop3/drivers/block/loop.c Mon Jun 9 10:39:45 2003
@@ -439,21 +439,22 @@
return 0;
}
-static struct bio *bio_copy(struct bio *bio, int gfp_mask, int copy)
+static struct bio *loop_copy_bio(struct bio *rbh, int gfp_mask, int copy)
{
- struct bio *b = bio_alloc(gfp_mask, bio->bi_vcnt);
+ struct bio *bio;
unsigned long flags = 0; /* gcc silly */
struct bio_vec *bv;
int i;
- if (unlikely(!b))
+ bio = bio_alloc(gfp_mask, rbh->bi_vcnt);
+ if (!bio)
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];
+ __bio_for_each_segment(bv, rbh, i, 0) {
+ struct bio_vec *bbv = &bio->bi_io_vec[i];
char *vfrom, *vto;
bbv->bv_page = alloc_page(gfp_mask);
@@ -490,18 +491,18 @@
}
}
- b->bi_sector = bio->bi_sector;
- b->bi_bdev = bio->bi_bdev;
- b->bi_rw = bio->bi_rw;
+ bio->bi_sector = rbh->bi_sector;
+ bio->bi_bdev = rbh->bi_bdev;
+ bio->bi_rw = rbh->bi_rw;
- b->bi_vcnt = bio->bi_vcnt;
- b->bi_size = bio->bi_size;
+ bio->bi_vcnt = rbh->bi_vcnt;
+ bio->bi_size = rbh->bi_size;
- return b;
+ return bio;
oom:
while (--i >= 0)
- __free_page(b->bi_io_vec[i].bv_page);
+ __free_page(bio->bi_io_vec[i].bv_page);
bio_put(bio);
return NULL;
@@ -529,7 +530,8 @@
int flags = current->flags;
current->flags &= ~PF_MEMALLOC;
- bio = bio_copy(rbh, (GFP_ATOMIC & ~__GFP_HIGH) | __GFP_NOWARN,
+ bio = loop_copy_bio(rbh,
+ (GFP_ATOMIC & ~__GFP_HIGH) | __GFP_NOWARN,
rbh->bi_rw & WRITE);
current->flags = flags;
if (bio == NULL)
@@ -547,9 +549,8 @@
return bio;
}
-static int
-bio_transfer(struct loop_device *lo, struct bio *to_bio,
- struct bio *from_bio)
+static int loop_transfer_bio(struct loop_device *lo,
+ struct bio *to_bio, struct bio *from_bio)
{
unsigned long IV = loop_get_iv(lo, from_bio->bi_sector);
struct bio_vec *from_bvec, *to_bvec;
@@ -614,7 +615,7 @@
new_bio = loop_get_buffer(lo, old_bio);
IV = loop_get_iv(lo, old_bio->bi_sector);
if (rw == WRITE) {
- if (bio_transfer(lo, new_bio, old_bio))
+ if (loop_transfer_bio(lo, new_bio, old_bio))
goto err;
}
@@ -646,7 +647,7 @@
} else {
struct bio *rbh = bio->bi_private;
- ret = bio_transfer(lo, bio, rbh);
+ ret = loop_transfer_bio(lo, bio, rbh);
bio_endio(rbh, rbh->bi_size, ret);
loop_put_buffer(bio);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 4/9 copy bio not data
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
2003-06-10 15:31 ` [PATCH] loop 2/9 absorb bio_copy Hugh Dickins
2003-06-10 15:31 ` [PATCH] loop 3/9 loop bio renaming Hugh Dickins
@ 2003-06-10 15:32 ` Hugh Dickins
2003-06-10 15:33 ` [PATCH] loop 5/9 remove an IV Hugh Dickins
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:32 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
Remove copy flag and code from loop_copy_bio: wasn't used when reading,
and waste of time when writing - the loop transfer function does that.
And don't initialize bio fields immediately reinitialized by caller.
--- loop3/drivers/block/loop.c Mon Jun 9 10:39:45 2003
+++ loop4/drivers/block/loop.c Mon Jun 9 10:44:49 2003
@@ -439,10 +439,9 @@
return 0;
}
-static struct bio *loop_copy_bio(struct bio *rbh, int gfp_mask, int copy)
+static struct bio *loop_copy_bio(struct bio *rbh, int gfp_mask)
{
struct bio *bio;
- unsigned long flags = 0; /* gcc silly */
struct bio_vec *bv;
int i;
@@ -451,11 +450,10 @@
return NULL;
/*
- * iterate iovec list and alloc pages + copy data
+ * iterate iovec list and alloc pages
*/
__bio_for_each_segment(bv, rbh, i, 0) {
struct bio_vec *bbv = &bio->bi_io_vec[i];
- char *vfrom, *vto;
bbv->bv_page = alloc_page(gfp_mask);
if (bbv->bv_page == NULL)
@@ -463,38 +461,8 @@
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);
- }
}
- bio->bi_sector = rbh->bi_sector;
- bio->bi_bdev = rbh->bi_bdev;
- bio->bi_rw = rbh->bi_rw;
-
bio->bi_vcnt = rbh->bi_vcnt;
bio->bi_size = rbh->bi_size;
@@ -531,8 +499,7 @@
current->flags &= ~PF_MEMALLOC;
bio = loop_copy_bio(rbh,
- (GFP_ATOMIC & ~__GFP_HIGH) | __GFP_NOWARN,
- rbh->bi_rw & WRITE);
+ (GFP_ATOMIC & ~__GFP_HIGH) | __GFP_NOWARN);
current->flags = flags;
if (bio == NULL)
blk_congestion_wait(WRITE, HZ/10);
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 5/9 remove an IV
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
` (2 preceding siblings ...)
2003-06-10 15:32 ` [PATCH] loop 4/9 copy bio not data Hugh Dickins
@ 2003-06-10 15:33 ` Hugh Dickins
2003-06-10 15:34 ` [PATCH] loop 6/9 remove LO_FLAGS_BH_REMAP Hugh Dickins
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:33 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
Remove unused IV from loop_make_request (loop_transfer_bio does that).
--- loop4/drivers/block/loop.c Mon Jun 9 15:58:48 2003
+++ loop5/drivers/block/loop.c Mon Jun 9 15:57:31 2003
@@ -544,7 +544,6 @@
{
struct bio *new_bio = NULL;
struct loop_device *lo = q->queuedata;
- unsigned long IV;
int rw = bio_rw(old_bio);
if (!lo)
@@ -580,7 +579,6 @@
* piggy old buffer on original, and submit for I/O
*/
new_bio = loop_get_buffer(lo, old_bio);
- IV = loop_get_iv(lo, old_bio->bi_sector);
if (rw == WRITE) {
if (loop_transfer_bio(lo, new_bio, old_bio))
goto err;
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 6/9 remove LO_FLAGS_BH_REMAP
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
` (3 preceding siblings ...)
2003-06-10 15:33 ` [PATCH] loop 5/9 remove an IV Hugh Dickins
@ 2003-06-10 15:34 ` Hugh Dickins
2003-06-10 15:42 ` Jens Axboe
2003-06-10 15:35 ` [PATCH] loop 7/9 remove blk_queue_bounce Hugh Dickins
` (2 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:34 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
Jonah Sherman <jsherman@stuy.edu> pointed out back in February how
LO_FLAGS_BH_REMAP is never actually set, since loop_init_xfer only calls
the init for non-0 encryption type. Fix that or scrap it? Let's scrap
it for now, that path (hacking values in bio instead of copying data)
seems never to have been tested, and adds to the number of paths through
loop: leave that optimization to some other occasion.
--- loop5/drivers/block/loop.c Tue Jun 10 12:54:36 2003
+++ loop6/drivers/block/loop.c Tue Jun 10 12:56:34 2003
@@ -120,12 +120,6 @@
return 0;
}
-static int none_status(struct loop_device *lo, const struct loop_info64 *info)
-{
- lo->lo_flags |= LO_FLAGS_BH_REMAP;
- return 0;
-}
-
static int xor_status(struct loop_device *lo, const struct loop_info64 *info)
{
if (info->lo_encrypt_key_size <= 0)
@@ -136,7 +130,6 @@
struct loop_func_table none_funcs = {
.number = LO_CRYPT_NONE,
.transfer = transfer_none,
- .init = none_status,
};
struct loop_func_table xor_funcs = {
@@ -481,14 +474,6 @@
struct bio *bio;
/*
- * for xfer_funcs that can operate on the same bh, do that
- */
- if (lo->lo_flags & LO_FLAGS_BH_REMAP) {
- bio = rbh;
- goto out_bh;
- }
-
- /*
* When called on the page reclaim -> writepage path, this code can
* trivially consume all memory. So we drop PF_MEMALLOC to avoid
* stealing all the page reserves and throttle to the writeout rate.
@@ -507,8 +492,6 @@
bio->bi_end_io = loop_end_io_transfer;
bio->bi_private = rbh;
-
-out_bh:
bio->bi_sector = rbh->bi_sector + (lo->lo_offset >> 9);
bio->bi_rw = rbh->bi_rw;
bio->bi_bdev = lo->lo_device;
--- loop5/include/linux/loop.h Sun Apr 20 08:02:13 2003
+++ loop6/include/linux/loop.h Tue Jun 10 12:56:08 2003
@@ -70,7 +70,6 @@
*/
#define LO_FLAGS_DO_BMAP 1
#define LO_FLAGS_READ_ONLY 2
-#define LO_FLAGS_BH_REMAP 4
#include <asm/posix_types.h> /* for __kernel_old_dev_t */
#include <asm/types.h> /* for __u64 */
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 7/9 remove blk_queue_bounce
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
` (4 preceding siblings ...)
2003-06-10 15:34 ` [PATCH] loop 6/9 remove LO_FLAGS_BH_REMAP Hugh Dickins
@ 2003-06-10 15:35 ` Hugh Dickins
2003-06-10 15:37 ` [PATCH] loop 8/9 copy_bio use highmem Hugh Dickins
2003-06-10 15:38 ` [PATCH] loop 9/9 don't lose PF_MEMDIE Hugh Dickins
7 siblings, 0 replies; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
What purpose does loop_make_request's blk_queue_bounce serve? None,
it's just a relic from before the kmaps were added to loop's transfers,
and ties up mempooled resources - in the file-backed case, with no
guarantee they'll soon be freed. And what purpose does loop_set_fd's
blk_queue_bounce_limit serve? None, blk_queue_make_request did that.
--- loop6/drivers/block/loop.c Tue Jun 10 12:56:34 2003
+++ loop7/drivers/block/loop.c Tue Jun 10 12:59:47 2003
@@ -548,8 +548,6 @@
goto err;
}
- blk_queue_bounce(q, &old_bio);
-
/*
* file backed, queue for loop_thread to handle
*/
@@ -742,7 +740,6 @@
* device
*/
blk_queue_make_request(&lo->lo_queue, loop_make_request);
- blk_queue_bounce_limit(&lo->lo_queue, BLK_BOUNCE_HIGH);
lo->lo_queue.queuedata = lo;
/*
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 8/9 copy_bio use highmem
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
` (5 preceding siblings ...)
2003-06-10 15:35 ` [PATCH] loop 7/9 remove blk_queue_bounce Hugh Dickins
@ 2003-06-10 15:37 ` Hugh Dickins
2003-06-10 15:38 ` [PATCH] loop 9/9 don't lose PF_MEMDIE Hugh Dickins
7 siblings, 0 replies; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:37 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
loop_copy_bio uses one gfp_mask for bio_alloc and alloc_page calls.
The bio_alloc obviously can't use highmem, but the alloc_page can.
Yes, the underlying device might be unable to use highmem, and have
to use one of its bounce buffers, with an extra copy: so be it.
(Originally I did propagate the underlying device's bounce needs down to
the loop device, to avoid that possible extra copy; but let's keep this
simple, the low end doesn't have highmem and the high end can I/O it.)
--- loop7/drivers/block/loop.c Tue Jun 10 11:40:23 2003
+++ loop8/drivers/block/loop.c Tue Jun 10 11:55:11 2003
@@ -432,13 +432,13 @@
return 0;
}
-static struct bio *loop_copy_bio(struct bio *rbh, int gfp_mask)
+static struct bio *loop_copy_bio(struct bio *rbh)
{
struct bio *bio;
struct bio_vec *bv;
int i;
- bio = bio_alloc(gfp_mask, rbh->bi_vcnt);
+ bio = bio_alloc(__GFP_NOWARN, rbh->bi_vcnt);
if (!bio)
return NULL;
@@ -448,7 +448,7 @@
__bio_for_each_segment(bv, rbh, i, 0) {
struct bio_vec *bbv = &bio->bi_io_vec[i];
- bbv->bv_page = alloc_page(gfp_mask);
+ bbv->bv_page = alloc_page(__GFP_NOWARN|__GFP_HIGHMEM);
if (bbv->bv_page == NULL)
goto oom;
@@ -483,8 +483,7 @@
int flags = current->flags;
current->flags &= ~PF_MEMALLOC;
- bio = loop_copy_bio(rbh,
- (GFP_ATOMIC & ~__GFP_HIGH) | __GFP_NOWARN);
+ bio = loop_copy_bio(rbh);
current->flags = flags;
if (bio == NULL)
blk_congestion_wait(WRITE, HZ/10);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] loop 2/9 absorb bio_copy
2003-06-10 15:31 ` [PATCH] loop 2/9 absorb bio_copy Hugh Dickins
@ 2003-06-10 15:37 ` Jens Axboe
2003-06-10 16:02 ` Hugh Dickins
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2003-06-10 15:37 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] loop 9/9 don't lose PF_MEMDIE
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
` (6 preceding siblings ...)
2003-06-10 15:37 ` [PATCH] loop 8/9 copy_bio use highmem Hugh Dickins
@ 2003-06-10 15:38 ` Hugh Dickins
7 siblings, 0 replies; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 15:38 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
loop_get_buffer loses PF_MEMDIE if it's added while in loop_copy_bio:
not a high probability since it's not waiting there, but could happen,
and sets a bad example (compare with add_to_swap fixed a while back).
--- loop8/drivers/block/loop.c Tue Jun 10 11:55:11 2003
+++ loop9/drivers/block/loop.c Tue Jun 10 12:05:17 2003
@@ -484,7 +484,9 @@
current->flags &= ~PF_MEMALLOC;
bio = loop_copy_bio(rbh);
- current->flags = flags;
+ if (flags & PF_MEMALLOC)
+ current->flags |= PF_MEMALLOC;
+
if (bio == NULL)
blk_congestion_wait(WRITE, HZ/10);
} while (bio == NULL);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] loop 6/9 remove LO_FLAGS_BH_REMAP
2003-06-10 15:34 ` [PATCH] loop 6/9 remove LO_FLAGS_BH_REMAP Hugh Dickins
@ 2003-06-10 15:42 ` Jens Axboe
0 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2003-06-10 15:42 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, linux-kernel
On Tue, Jun 10 2003, Hugh Dickins wrote:
> Jonah Sherman <jsherman@stuy.edu> pointed out back in February how
> LO_FLAGS_BH_REMAP is never actually set, since loop_init_xfer only calls
> the init for non-0 encryption type. Fix that or scrap it? Let's scrap
> it for now, that path (hacking values in bio instead of copying data)
> seems never to have been tested, and adds to the number of paths through
> loop: leave that optimization to some other occasion.
I agree with scrapping it, it's only really interesting for non-transfer
block direct remaps which can be done with dm or similar.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] loop 2/9 absorb bio_copy
2003-06-10 16:02 ` Hugh Dickins
@ 2003-06-10 16:01 ` Jens Axboe
0 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2003-06-10 16:01 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] loop 2/9 absorb bio_copy
2003-06-10 15:37 ` Jens Axboe
@ 2003-06-10 16:02 ` Hugh Dickins
2003-06-10 16:01 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Hugh Dickins @ 2003-06-10 16:02 UTC (permalink / raw)
To: Jens Axboe; +Cc: Andrew Morton, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2003-06-10 15:48 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-06-10 15:30 [PATCH] loop 1/9 file use highmem Hugh Dickins
2003-06-10 15:31 ` [PATCH] loop 2/9 absorb bio_copy Hugh Dickins
2003-06-10 15:37 ` Jens Axboe
2003-06-10 16:02 ` Hugh Dickins
2003-06-10 16:01 ` Jens Axboe
2003-06-10 15:31 ` [PATCH] loop 3/9 loop bio renaming Hugh Dickins
2003-06-10 15:32 ` [PATCH] loop 4/9 copy bio not data Hugh Dickins
2003-06-10 15:33 ` [PATCH] loop 5/9 remove an IV Hugh Dickins
2003-06-10 15:34 ` [PATCH] loop 6/9 remove LO_FLAGS_BH_REMAP Hugh Dickins
2003-06-10 15:42 ` Jens Axboe
2003-06-10 15:35 ` [PATCH] loop 7/9 remove blk_queue_bounce Hugh Dickins
2003-06-10 15:37 ` [PATCH] loop 8/9 copy_bio use highmem Hugh Dickins
2003-06-10 15:38 ` [PATCH] loop 9/9 don't lose PF_MEMDIE Hugh Dickins
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox