| Message ID | 1486275733-7268-5-git-send-email-tom.leiming@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Feb 05, 2017 at 02:22:13PM +0800, Ming Lei wrote: > Firstly bio_clone_mddev() is used in raid normal I/O and isn't > in resync I/O path. > > Secondly all the direct access to bvec table in raid happens on > resync I/O except for write behind of raid1, in which we still > use bio_clone() for allocating new bvec table. > > So this patch replaces bio_clone() with bio_clone_fast() > in bio_clone_mddev(). Having the _fast in the name would be really useful for the reader. And as far as I can tell in the callers mddev is never NULL and neither is ->bio_set, so replacing bio_clone_mddev with raw calls to bio_clone_fast would be my preference.
On Mon, Feb 6, 2017 at 4:54 PM, Christoph Hellwig <hch@infradead.org> wrote: > On Sun, Feb 05, 2017 at 02:22:13PM +0800, Ming Lei wrote: >> Firstly bio_clone_mddev() is used in raid normal I/O and isn't >> in resync I/O path. >> >> Secondly all the direct access to bvec table in raid happens on >> resync I/O except for write behind of raid1, in which we still >> use bio_clone() for allocating new bvec table. >> >> So this patch replaces bio_clone() with bio_clone_fast() >> in bio_clone_mddev(). > > Having the _fast in the name would be really useful for the > reader. And as far as I can tell in the callers mddev is never > NULL and neither is ->bio_set, so replacing bio_clone_mddev with In theory, ->bio_set still might be NULL in case of failed memory allocation, please see md_run(). > raw calls to bio_clone_fast would be my preference. Thanks, Ming Lei
On Mon, Feb 06, 2017 at 06:43:32PM +0800, Ming Lei wrote: > In theory, ->bio_set still might be NULL in case of failed memory allocation, > please see md_run(). And that's something that should be fixed. Silently not having mempool is very bad behavior.
diff --git a/drivers/md/md.c b/drivers/md/md.c index 704be11355a9..7d176f025add 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -193,10 +193,14 @@ EXPORT_SYMBOL_GPL(bio_alloc_mddev); struct bio *bio_clone_mddev(struct bio *bio, gfp_t gfp_mask, struct mddev *mddev) { + struct bio_set *bs; + if (!mddev || !mddev->bio_set) - return bio_clone(bio, gfp_mask); + bs = fs_bio_set; + else + bs = mddev->bio_set; - return bio_clone_bioset(bio, gfp_mask, mddev->bio_set); + return bio_clone_fast(bio, gfp_mask, bs); } EXPORT_SYMBOL_GPL(bio_clone_mddev);
Firstly bio_clone_mddev() is used in raid normal I/O and isn't in resync I/O path. Secondly all the direct access to bvec table in raid happens on resync I/O except for write behind of raid1, in which we still use bio_clone() for allocating new bvec table. So this patch replaces bio_clone() with bio_clone_fast() in bio_clone_mddev(). Signed-off-by: Ming Lei <tom.leiming@gmail.com> --- drivers/md/md.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)