linux-kernel - Re: [PATCH #tj-block-for-linus] block: fix failfast merge testing in elv_rq_merge

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: <4A5EE2BD.8030100@panasas.com>
Date:	Thu, 16 Jul 2009 11:20:13 +0300
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Tejun Heo <tj@...nel.org>
CC:	Linux Kernel <linux-kernel@...r.kernel.org>,
	Jens Axboe <jens.axboe@...cle.com>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Jeff Garzik <jeff@...zik.org>
Subject: Re: [PATCH #tj-block-for-linus] block: fix failfast merge testing
 in elv_rq_merge_ok()

On 07/16/2009 11:06 AM, Tejun Heo wrote:
> Boaz Harrosh wrote:
>> On 07/16/2009 09:44 AM, Tejun Heo wrote:
>>> Commit ab0fd1debe730ec9998678a0c53caefbd121ed10 tries to prevent merge
>>> of requests with different failfast settings.  In elv_rq_merge_ok(),
>>> it compares new bio's failfast flags against the merge target
>>> request's.  However, the flag testing accessors for bio and blk don't
>>> return boolean but the tested bit value directly and FAILFAST on bio
>>> and blk don't match, so directly comparing them with == results in
>>> false negative unnecessary preventing merge of readahead requests.
>>>
>>> This patch convert the results to boolean by negating them before
>>> comparison.
>> I don't like that at all. Please fix the accessors to return
>> boolean. They look and regarded as boolean. I've never seen
>> them used as their bit value.
> 
> Yeah, I'll be happier that way but please note that this patch is only
> for 2.6.31.  2.6.32 won't have this code at all and we're past the
> merge window, so the smallest fix wins in this case, I think.  Also,
> changing only some of the accessors will increase the level of
> confusion while changing all of them for 2.6.31 at this point is way
> too invasive (there can be cases where the bit mask return value is
> depended upon).
> 

OK So could you put a FIXME: and fat comment, on that weird "!"(s)
everywhere?

> Looks like the flags are gonna go through considerable cleanup pretty
> soon, so let's postpone small things till then.
> 
>> if you are concerned with performance don't
>> an if(flag & bit) is even slightly slower then
>>    if(0 != (flag & bit)) on some processors
> 
> I wasn't worried about the performance at all.
> 
> Thanks.
> 

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