Hi Joe and Akira,
At Sat, 19 Jul 2014 11:13:13 +0900,
Satoru Takeuchi wrote:
>
> Hi Akira,
>
> 2014-07-18 10:01 GMT+09:00 Akira Hayakawa <ruby.wktk@xxxxxxxxx>:
> > Hi, Satoru,
> >
> > I totally agree with removing the BUG() at (2).
> >
> > However, I don't agree with setting the upper limit of nr_rambuf_pool.
> > You are saying that any memory allocation failure on initialization should be avoided
> > but that sounds going too far. Removing that possibility from all kernel codes is impossible in practice.
> > Carefully terminating the initialization on allocation failure is sufficient.
Both Akira and me seem to have completely different policy about the upper
limit of nr_rambuf_pool argument. However both of us agree with removing
unsure BUG() in int init_rambuf_pool(). So I wrote a patch to do so as a
first step. Please apply this patch.
In addition, I'd like to know your opinion about whether setting
the upper limit of nr_rambuf_pool argument is neccesary or not.
=======
From: Satoru Takeuchi <satoru.takeuchi@xxxxxxxxx>
If users set a very large value to nr_rambuf_pool argument, kernel would hit
BUG() in the error route of init_rambuf_pool().
drivers/md/dm-writeboost-metadata.c:
===============================================================================
...
static int init_rambuf_pool(struct wb_device *wb)
{
int r = 0;
size_t i;
wb->rambuf_pool = kmalloc(sizeof(struct rambuffer) * wb->nr_rambuf_pool,
GFP_KERNEL);
if (!wb->rambuf_pool) # It is true here.
return -ENOMEM;
wb->rambuf_cachep = kmem_cache_create("dmwb_rambuf",
1 << (wb->segment_size_order + SECTOR_SHIFT),
1 << (wb->segment_size_order + SECTOR_SHIFT),
SLAB_RED_ZONE, NULL);
if (!wb->rambuf_cachep) {
r = -ENOMEM; # Enter this route or ....
goto bad_cachep;
}
for (i = 0; i < wb->nr_rambuf_pool; i++) {
size_t j;
struct rambuffer *rambuf = wb->rambuf_pool + i;
rambuf->data = kmem_cache_alloc(wb->rambuf_cachep, GFP_KERNEL);
if (!rambuf->data) {
... # ... enter this route.
goto bad_alloc_data;
}
check_buffer_alignment(rambuf->data);
}
return r;
bad_alloc_data:
kmem_cache_destroy(wb->rambuf_cachep);
bad_cachep:
kfree(wb->rambuf_pool);
BUG(); # Kernel panic happens here.
return r;
}
...
===============================================================================
Probably this BUG() was introduced erroneously and is safe to remove.
Signed-off-by: Satoru Takeuchi <satoru.takeuchi@xxxxxxxxx>
Cc: Joe Thornber <ejt@xxxxxxxxxx>
Cc: Akira Hayakawa <ruby.wktk@xxxxxxxxx>
---
drivers/md/dm-writeboost-metadata.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/md/dm-writeboost-metadata.c b/drivers/md/dm-writeboost-metadata.c
index b7b3eb7..ce6ea056 100644
--- a/drivers/md/dm-writeboost-metadata.c
+++ b/drivers/md/dm-writeboost-metadata.c
@@ -702,7 +702,6 @@ bad_alloc_data:
kmem_cache_destroy(wb->rambuf_cachep);
bad_cachep:
kfree(wb->rambuf_pool);
- BUG();
return r;
}
--
2.0.1
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
