Re: Re: [PATCH 2/2] raid5: For write performance, remove REQ_SYNC when write was odirect.
From: majianpeng <hidden>
Date: 2012-07-16 06:41:50
Also in:
linux-fsdevel
On 2012-07-16 13:40 NeilBrown [off-list ref] Wrote:
On Mon, 16 Jul 2012 09:31:55 +0800 majianpeng [off-list ref] wrote:quoted
In commit e9c7469bb4f502dafc092166201bea1ad5fc0fbf: Tejun Heo introduced "implment REQ_FLUSH/FUA support". But for direct-write-blocks, it maybe for other purpose which like the regular file. And this flag will set STRIPE_PREREAD_ACTIVE which decreaed the change to full write. But this patch remove REQ_SYNC only judging the WRITE_ODIRECT,it will contail regular file.So it maybe not correctly. How can difference odriect_write between regular file or block file?Hi, I think you are saying the when REQ_SYNC is used with O_DIRECT writes it is having a negative effect on throughput because it allows the stripe to be processed immediately without waiting for more requests to be added to the stripe. Normal 'sync' requests use WRITE_SYNC which includes "REQ_NOIDLE" which means /* don't anticipate more IO after this one */ O_DIRECT request use WRITE_ODIRECT which does not include this flag.
Using REQ_NOIDEL to difference odirect and sync.Why not using: + if (bi->bi_rw & WRITE_ODIRECT) + bi->bi_rw &= ~REQ_SYNC; The flag WRITE_ODIRECT is only used in odirect-write.
So maybe we should simply change raid5 to only set STRIPE_PREREAD_ACTIVE if REQ_NOIDLE is set on the bio. I think this would have the same effect as what you are trying to achieve. Could you please try that and see if it has the desired effect on performance?
I tested and the performance is the same.
quoted hunk ↗ jump to hunk
Thanks, NeilBrown i.e. something like this:diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index d56d74d..2d72a57 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c@@ -4178,7 +4178,7 @@ static void make_request(struct mddev *mddev, struct bio * bi)finish_wait(&conf->wait_for_overlap, &w); set_bit(STRIPE_HANDLE, &sh->state); clear_bit(STRIPE_DELAYED, &sh->state); - if ((bi->bi_rw & REQ_SYNC) && + if ((bi->bi_rw & REQ_NOIDLE) && !test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) atomic_inc(&conf->preread_active_stripes); release_stripe_plug(mddev, sh);quoted
Signed-off-by: Jianpeng Ma <redacted> --- drivers/md/raid5.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 04348d7..8d2d4d1 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c@@ -4010,6 +4010,9 @@ static void make_request(struct mddev *mddev, struct bio * bi) chunk_aligned_read(mddev,bi)) return; + if (bi->bi_rw & WRITE_ODIRECT) + bi->bi_rw &= ~REQ_SYNC; + logical_sector = bi->bi_sector & ~((sector_t)STRIPE_SECTORS-1); last_sector = bi->bi_sector + (bi->bi_size>>9); bi->bi_next = NULL;