linux-kernel - Re: [PATCH 2/3] block: prevent duplicated bio completion report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F161E43.5040705@gmail.com>
Date:	Wed, 18 Jan 2012 10:20:03 +0900
From:	Namhyung Kim <namhyung@...il.com>
To:	linux-kernel@...r.kernel.org
Cc:	Namhyung Kim <namhyung.kim@....com>, Jens Axboe <axboe@...nel.dk>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] block: prevent duplicated bio completion report

Hi,

2012-01-18 2:45 AM, Tejun Heo wrote:
> Hello,
>
> On Tue, Jan 17, 2012 at 10:32:07AM +0900, Namhyung Kim wrote:
>> Since previous patch make block_bio_complete TP working,
>> it will generate duplicated BLK_TA_COMPLETEs for bounced
>> bios. Fix it.
>>
>> Signed-off-by: Namhyung Kim <namhyung.kim@....com>
>> Cc: Tejun Heo <tj@...nel.org>
>
> This and the third patch should probably be merged to the first patch.
> As it currently stands, it introduces window where spurious events are
> generated.

OK, will do.


>
>> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>> index 4053cbd4490e..45cd0074a1c8 100644
>> --- a/include/linux/blk_types.h
>> +++ b/include/linux/blk_types.h
>> @@ -97,6 +97,9 @@ struct bio {
>>   #define BIO_MAPPED_INTEGRITY 11/* integrity metadata has been remapped */
>>   #define bio_flagged(bio, flag)	((bio)->bi_flags&  (1<<  (flag)))
>>
>> +/* masked bio's won't report its completion via tracepoint */
>> +#define BIO_COMPLETE_MASK	(1<<  BIO_BOUNCED)
>
> And, who's setting this flag?

__blk_queue_bounce() does.


>
>>   /*
>>    * top 4 bits of bio flags indicate the pool this bio came from
>>    */
>> diff --git a/include/trace/events/block.h b/include/trace/events/block.h
>> index 96955f4828b3..72888542e186 100644
>> --- a/include/trace/events/block.h
>> +++ b/include/trace/events/block.h
>> @@ -219,7 +219,8 @@ TRACE_EVENT_CONDITION(block_bio_complete,
>>
>>   	TP_ARGS(q, bio, error),
>>
>> -	TP_CONDITION(bio->bi_bdev != NULL),
>> +	TP_CONDITION(bio->bi_bdev != NULL&&
>> +		     !(bio->bi_flags&  BIO_COMPLETE_MASK)),
>
> Bounced bio's are separate bio's too and I don't think masking its
> completion from the TP itself is a good idea.  As I wrote before, why
> not do this from blktrace code?

Because blktrace cannot know about the bi_flags, as I said before. :) 
And although the bounced bio's are separate ones, they aren't queued 
separately. They just get replaced on the way.

Besides, I think accounting wait_time of them will result in an invalid 
value unless it's handled somehow in block_bio_bounce and/or 
block_rq_issue TP.


Thanks,
Namhyung

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/