fs/direct-io.c: Set bi_rw when alloc bio.
RFC PATCH 1/1 fs/direct-io.c: Set bi_rw when alloc bio.
When exec bio_alloc, the bi_rw is zero.But after calling bio_add_page, it will use bi_rw.
Fox example, in functiion __bio_add_page,it will call merge_bvec_fn(). The merge_bvec_fn of raid456 will use the bi_rw to judge the merge.
Quoted 2 lines
Quoted 2 lines
if ((
bvm->bi_rw& 1) == WRITE)return biovec->bv_len; /* always allow writes to be mergeable */
There are many place like this in kernel.If you think this patch ok, i will correct those.
--- fs/direct-io.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
fs/direct-io.c +1 -0
@@ -349,6 +349,7 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio, bio->bi_bdev = bdev; bio->bi_sector = first_sector; + bio->bi_rw = dio->rw;if (dio->is_async) bio->bi_end_io = dio_bio_end_aio;else
Signature · 1 line
1.7.5.4
Quoted 8 lines · majianpeng
When exec
bio_alloc, thebi_rwis zero.But after callingbio_add_page, it will usebi_rw.
Fox example, in functiion __bio_add_page,it will callmerge_bvec_fn(). Themerge_bvec_fnof raid456 will use thebi_rwto judge the merge.Quoted 2 lines
Quoted 2 lines
if ((
bvm->bi_rw& 1) == WRITE)return biovec->bv_len; /* always allow writes to be mergeable */Signed-off-by Jianpeng Ma <[email protected]>
Good catch. How did you find this? Did you experience data corruption as a result of this oversight, reduced performance due to missed merge opportunities, or did you just notice it in reviewing the code?
Quoted 18 lines
There are many place like this in kernel.If you think this patch ok, i will correct those.
--- fs/direct-io.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)fs/direct-io.c +1
-0@@ -349,6 +349,7 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio, bio->bi_bdev = bdev; bio->bi_sector = first_sector; + bio->bi_rw = dio->rw;if (dio->is_async) bio->bi_end_io = dio_bio_end_aio;else
On 2012-07-27 22:21 Jeff Moyer <[email protected]> Wrote:
Quoted 9 lines
Quoted 7 lines · majianpeng
When exec
bio_alloc, thebi_rwis zero.But after callingbio_add_page, it will usebi_rw.
Fox example, in functiion __bio_add_page,it will callmerge_bvec_fn(). Themerge_bvec_fnof raid456 will use thebi_rwto judge the merge.Quoted 2 lines
Quoted 2 lines
if ((
bvm->bi_rw& 1) == WRITE)return biovec->bv_len; /* always allow writes to be mergeable */
Quoted 8 lines
Quoted 1 line
Signed-off-by Jianpeng Ma <[email protected]>Good catch. How did you find this? Did you experience data corruption as a result of this oversight, reduced performance due to missed merge opportunities, or did you just notice it in reviewing the code?
Reviewed-by Jeff Moyer <[email protected]>
Sorry for late to reply. When i analysed the performance of raid5, i found this bug.
Quoted 18 lines
Quoted 18 lines
There are many place like this in kernel.If you think this patch ok, i will correct those.
--- fs/direct-io.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)fs/direct-io.c +1
-0@@ -349,6 +349,7 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio, bio->bi_bdev = bdev; bio->bi_sector = first_sector; + bio->bi_rw = dio->rw;if (dio->is_async) bio->bi_end_io = dio_bio_end_aio; else�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay� 蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��� �鹅�⒏?朓�
Quoted 12 lines · majianpeng
On 2012-07-27 22:21 Jeff Moyer <[email protected]> Wrote:
Quoted 11 lines
Quoted 8 lines · majianpeng
When exec
bio_alloc, thebi_rwis zero.But after callingbio_add_page, it will usebi_rw.
Fox example, in functiion __bio_add_page,it will callmerge_bvec_fn(). Themerge_bvec_fnof raid456 will use thebi_rwto judge the merge.Quoted 2 lines
Quoted 2 lines
if ((
bvm->bi_rw& 1) == WRITE)return biovec->bv_len; /* always allow writes to be mergeable */Signed-off-by Jianpeng Ma <[email protected]>
Quoted 7 lines
Quoted 6 lines
Good catch. How did you find this? Did you experience data corruption as a result of this oversight, reduced performance due to missed merge opportunities, or did you just notice it in reviewing the code?
Reviewed-by Jeff Moyer <[email protected]>Sorry for late to reply. When i analysed the performance of raid5, i found this bug.
OK, thanks. In the future, it would be good to include that information in the patch description.
Cheers,
Jeff