[PATCH 1/2] blktrace: treat flush as barrier

[PATCH 1/2] blktrace: treat flush as barrier

[Namhyung Kim]

Since BARRIER requests have been converted to FLUSH/FUA, it would be
better for blktrace to recognize FLUSH requests as BARRIER for the
backward-compatibility IMHO.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
---
 kernel/trace/blktrace.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 6957aa298dfa..8635d332c50b 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -176,6 +176,7 @@ static const u32 ddir_act[2] = { BLK_TC_ACT(BLK_TC_READ),
 				 BLK_TC_ACT(BLK_TC_WRITE) };
 
 #define BLK_TC_RAHEAD		BLK_TC_AHEAD
+#define BLK_TC_FLUSH		BLK_TC_BARRIER
 
 /* The ilog2() calls fall out because they're constant */
 #define MASK_TC_BIT(rw, __name) ((rw & REQ_ ## __name) << \
@@ -206,6 +207,7 @@ static void __blk_add_trace(struct blk_trace *bt, sector_t sector, int bytes,
 	what |= MASK_TC_BIT(rw, RAHEAD);
 	what |= MASK_TC_BIT(rw, META);
 	what |= MASK_TC_BIT(rw, DISCARD);
+	what |= MASK_TC_BIT(rw, FLUSH);
 
 	pid = tsk->pid;
 	if (act_log_check(bt, what, sector, pid))
-- 
1.7.5.2

[PATCH 2/2] block: add REQ_SECURE to REQ_COMMON_MASK

[Namhyung Kim]

Add REQ_SECURE flag to REQ_COMMON_MASK so that
init_request_from_bio() can pass it to @req->cmd_flags.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Adrian Hunter <adrian.hunter@nokia.com>
---
I couldn't find where the flag is passed from bio to req and don't know
how mmc - it seems that it's the only user of the flag, AFAICS - works
by now. Maybe I'm missing something. :)

 include/linux/blk_types.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index be50d9e70a7d..acdb1436dc14 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -168,7 +168,7 @@ enum rq_flag_bits {
 	(REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
 #define REQ_COMMON_MASK \
 	(REQ_WRITE | REQ_FAILFAST_MASK | REQ_SYNC | REQ_META | REQ_DISCARD | \
-	 REQ_NOIDLE | REQ_FLUSH | REQ_FUA)
+	 REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE)
 #define REQ_CLONE_MASK		REQ_COMMON_MASK
 
 #define REQ_RAHEAD		(1 << __REQ_RAHEAD)
-- 
1.7.5.2

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Christoph Hellwig]

On Fri, May 27, 2011 at 10:11:22PM +0900, Namhyung Kim wrote:
> Since BARRIER requests have been converted to FLUSH/FUA, it would be
> better for blktrace to recognize FLUSH requests as BARRIER for the
> backward-compatibility IMHO.

I'd rather see new flags for them.  F and U maybe?

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Mike Snitzer]

On Fri, May 27, 2011 at 9:12 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, May 27, 2011 at 10:11:22PM +0900, Namhyung Kim wrote:
>> Since BARRIER requests have been converted to FLUSH/FUA, it would be
>> better for blktrace to recognize FLUSH requests as BARRIER for the
>> backward-compatibility IMHO.
>
> I'd rather see new flags for them.  F and U maybe?

Somehow I'm not surprised by your F and U suggestion -- appropriate on
multiple levels :)

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Namhyung Kim]

2011-05-27 (금), 09:57 -0400, Mike Snitzer:
> On Fri, May 27, 2011 at 9:12 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Fri, May 27, 2011 at 10:11:22PM +0900, Namhyung Kim wrote:
> >> Since BARRIER requests have been converted to FLUSH/FUA, it would be
> >> better for blktrace to recognize FLUSH requests as BARRIER for the
> >> backward-compatibility IMHO.
> >
> > I'd rather see new flags for them.  F and U maybe?
> 
> Somehow I'm not surprised by your F and U suggestion -- appropriate on
> multiple levels :)

OK. I'll work on that direction.
Thanks.


-- 
Regards,
Namhyung Kim

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Jens Axboe]

On 2011-05-27 17:13, Namhyung Kim wrote:
> 2011-05-27 (금), 09:57 -0400, Mike Snitzer:
>> On Fri, May 27, 2011 at 9:12 AM, Christoph Hellwig <hch@infradead.org> wrote:
>>> On Fri, May 27, 2011 at 10:11:22PM +0900, Namhyung Kim wrote:
>>>> Since BARRIER requests have been converted to FLUSH/FUA, it would be
>>>> better for blktrace to recognize FLUSH requests as BARRIER for the
>>>> backward-compatibility IMHO.
>>>
>>> I'd rather see new flags for them.  F and U maybe?
>>
>> Somehow I'm not surprised by your F and U suggestion -- appropriate on
>> multiple levels :)
> 
> OK. I'll work on that direction.
> Thanks.

Agree on Christophs comments, we should not pretend they are the same
(since they are not). Since flush is a request on its own, F works
nicely. For FUA it's associated with a write, so F should work there too
indicating Write Fua (and easily humanly parsed as that, or Write
Flush). WU would look confusing.

-- 
Jens Axboe

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Christoph Hellwig]

On Fri, May 27, 2011 at 10:17:09PM +0200, Jens Axboe wrote:
> Agree on Christophs comments, we should not pretend they are the same
> (since they are not). Since flush is a request on its own, F works
> nicely. For FUA it's associated with a write, so F should work there too
> indicating Write Fua (and easily humanly parsed as that, or Write
> Flush). WU would look confusing.

REQ_FLUSH can also be set on a write bio, it only gets split at the
request level.  And even there we're at least pondering allowing it
to stay as part of the write for some paravirtualized storage protocols.

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Namhyung Kim]

2011-05-27 (금), 16:27 -0400, Christoph Hellwig:
> On Fri, May 27, 2011 at 10:17:09PM +0200, Jens Axboe wrote:
> > Agree on Christophs comments, we should not pretend they are the same
> > (since they are not). Since flush is a request on its own, F works
> > nicely. For FUA it's associated with a write, so F should work there too
> > indicating Write Fua (and easily humanly parsed as that, or Write
> > Flush). WU would look confusing.
> 
> REQ_FLUSH can also be set on a write bio, it only gets split at the
> request level.  And even there we're at least pondering allowing it
> to stay as part of the write for some paravirtualized storage protocols.
> 

Hi,

AFAIK FLUSH always precedes WRITE and then followed by FUA, so how about
using the same F for both of them and distinguishing by position?

 - WRITE:            W
 - WRITE_FLUSH:      FW
 - WRITE_FUA:        WF
 - WRITE_FLUSH_FUA:  FWF

Or using lower-case 'f' for FUA?

Thanks.


-- 
Regards,
Namhyung Kim

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Christoph Hellwig]

On Sat, May 28, 2011 at 11:09:41AM +0900, Namhyung Kim wrote:
> AFAIK FLUSH always precedes WRITE and then followed by FUA, so how about

Right now most do because that's how the old barriers worked, but it's
going to change.  For $NEXT + 1 I have a patch that for will make
xfs sends FUA without flushes in a lot of cases, and afaik some device
mapper code already does now.

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Namhyung Kim]

2011-05-28 (토), 03:44 -0400, Christoph Hellwig:
> On Sat, May 28, 2011 at 11:09:41AM +0900, Namhyung Kim wrote:
> > AFAIK FLUSH always precedes WRITE and then followed by FUA, so how about
> 
> Right now most do because that's how the old barriers worked, but it's
> going to change.  For $NEXT + 1 I have a patch that for will make
> xfs sends FUA without flushes in a lot of cases, and afaik some device
> mapper code already does now.
> 

OK, thanks for the explanation.

Btw, the point was it seems possible that we can use the 'F' for both
FLUSH and FUA if we distinguish them by their (relative) position in the
output. How do you think? Do you still prefer F/U flags?

Thanks.


-- 
Regards,
Namhyung Kim

Re: [PATCH 1/2] blktrace: treat flush as barrier

[Jens Axboe]

On 2011-05-28 04:09, Namhyung Kim wrote:
> 2011-05-27 (금), 16:27 -0400, Christoph Hellwig:
>> On Fri, May 27, 2011 at 10:17:09PM +0200, Jens Axboe wrote:
>>> Agree on Christophs comments, we should not pretend they are the same
>>> (since they are not). Since flush is a request on its own, F works
>>> nicely. For FUA it's associated with a write, so F should work there too
>>> indicating Write Fua (and easily humanly parsed as that, or Write
>>> Flush). WU would look confusing.
>>
>> REQ_FLUSH can also be set on a write bio, it only gets split at the
>> request level.  And even there we're at least pondering allowing it
>> to stay as part of the write for some paravirtualized storage protocols.
>>
> 
> Hi,
> 
> AFAIK FLUSH always precedes WRITE and then followed by FUA, so how about
> using the same F for both of them and distinguishing by position?
> 
>  - WRITE:            W
>  - WRITE_FLUSH:      FW
>  - WRITE_FUA:        WF
>  - WRITE_FLUSH_FUA:  FWF

That looks fine. 'U' would be an illogical choice.

-- 
Jens Axboe