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 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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux