On Mon, 16 Jul 2012 15:11:29 +0800 majianpeng [off-list ref] wrote:

On 2012-07-16 15:07 NeilBrown [off-list ref] Wrote:

quoted

On Mon, 16 Jul 2012 14:42:54 +0800 majianpeng [off-list ref] wrote:

quoted

On 2012-07-16 13:40 NeilBrown [off-list ref] Wrote:

quoted

On Mon, 16 Jul 2012 09:31:55 +0800 majianpeng [off-list ref] wrote:

[snip]

quoted

quoted

quoted

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.

quoted

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