Re: Re: [PATCH 2/2] raid5: For write performance, remove REQ_SYNC when write was odirect.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 2012-07-16 15:30 NeilBrown <neilb@xxxxxxx> Wrote:
>On Mon, 16 Jul 2012 15:11:29 +0800 majianpeng <majianpeng@xxxxxxxxx> wrote:
>
>> On 2012-07-16 15:07 NeilBrown <neilb@xxxxxxx> Wrote:
>> >On Mon, 16 Jul 2012 14:42:54 +0800 majianpeng <majianpeng@xxxxxxxxx> wrote:
>> >
>> >> On 2012-07-16 13:40 NeilBrown <neilb@xxxxxxx> Wrote:
>> >> >On Mon, 16 Jul 2012 09:31:55 +0800 majianpeng <majianpeng@xxxxxxxxx> wrote:
>> >> >
>> [snip]
>> >> > 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;
>> >
>> >Because that code is wrong.  WRITE_ODIRECT is not one flag, it is two flags
>> >'or'ed together.  So this code does not do what you expect.
>> >
>> No, I used those code test and it's ok.
>> The code used & not &&.
>> Maybe I wrong?
>
>Think about it...
>
>#define REQ_WRITE               (1 << __REQ_WRITE)
>#define REQ_SYNC                (1 << __REQ_SYNC)
>
>#define RW_MASK                 REQ_WRITE
>#define WRITE                   RW_MASK
>
>#define WRITE_ODIRECT		(WRITE | REQ_SYNC)
>
>So   
>    (bi->bi_rw & WRITE_ODIRECT)
>
>will be true if either REQ_WRITE or REQ_SYNC are set in bi_rw
>So whenever REQ_SYNC is set, your code clears the flag.
>So your code is functionally identical to
>
>   bi->bi_rw &= ~REQ_SYNC;
>
>NeilBrown
>
>
Yes, thanks your time.I maked a stupid mistake.
I'll corrected by your suggestion and resend to you.?韬{.n?????%??檩??w?{.n???{炳盯w???塄}?财??j:+v??????2??璀??摺?囤??z夸z罐?+?????w棹f



[ATA RAID]     [Linux SCSI Target Infrastructure]     [Managing RAID on Linux]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device-Mapper]     [Kernel]     [Linux Books]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Photos]     [Yosemite Photos]     [Yosemite News]     [AMD 64]     [Linux Networking]

Add to Google Powered by Linux