2011-05-27 13:11:31

by Namhyung Kim

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

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 <[email protected]>
Cc: Tejun Heo <[email protected]>
---
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


2011-05-27 13:11:37

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/2] block: add REQ_SECURE to REQ_COMMON_MASK

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 <[email protected]>
Cc: Adrian Hunter <[email protected]>
---
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

2011-05-27 13:13:22

by Christoph Hellwig

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

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?

2011-05-27 13:58:06

by Mike Snitzer

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

On Fri, May 27, 2011 at 9:12 AM, Christoph Hellwig <[email protected]> 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 :)

2011-05-27 15:13:12

by Namhyung Kim

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

2011-05-27 (금), 09:57 -0400, Mike Snitzer:
> On Fri, May 27, 2011 at 9:12 AM, Christoph Hellwig <[email protected]> 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

2011-05-27 20:17:16

by Jens Axboe

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

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 <[email protected]> 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

2011-05-27 20:28:06

by Christoph Hellwig

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

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.

2011-05-28 02:09:49

by Namhyung Kim

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

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

2011-05-28 07:44:55

by Christoph Hellwig

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

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.

2011-05-31 10:27:28

by Namhyung Kim

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

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

2011-05-31 10:37:17

by Jens Axboe

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

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