linux-kernel - Re: [PATCH 2/4] block: use the same failfast bits for bio and request

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: <4A553DA4.4080408@kernel.org>
Date:	Thu, 09 Jul 2009 09:45:24 +0900
From:	Tejun Heo <tj@...nel.org>
To:	Boaz Harrosh <bharrosh@...asas.com>
CC:	Jens Axboe <jens.axboe@...cle.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	James Bottomley <James.Bottomley@...senPartnership.com>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	Niel Lambrechts <niel.lambrechts@...il.com>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH 2/4] block: use the same failfast bits for bio and request

Hello, Boaz.

Boaz Harrosh wrote:
> Thanks for doing this, it has been neglected for a long time.
> However, it will happen again, I don't like these implicit matches
> which are not enforced, They get to drift away. There are several ways
> to make sure two sets of enums stay in sync. (I'll have a try at it
> tomorrow. if you want). 

They don't share the exact same set of bits, so it's a bit blurry but
yeah it would be better if the bits are defined in more systematic
way.

>> @@ -142,37 +142,40 @@ struct bio {
>>   *
>>   * bit 0 -- data direction
>>   *	If not set, bio is a read from device. If set, it's a write to device.
>> - * bit 1 -- rw-ahead when set
>> - * bit 2 -- barrier
>> + * bit 1 -- fail fast device errors
>> + * bit 2 -- fail fast transport errors
>> + * bit 3 -- fail fast driver errors
>> + * bit 4 -- rw-ahead when set
>> + * bit 5 -- barrier
> 
> Please kill all these evil bit 1, bit 2 ,bit n comments. The ways we
> invent to torture ourselfs...
> 
> Just move all the comments to the enums declarations below. And be done
> with it, also for the next time.

Heh... I agree too.  Unless ABI is fixed, this type of comments are
often painful.  Care to submit a patch.  This series is already in
block#for-next.

>>  #define bio_rw_flagged(bio, flag)	((bio)->bi_rw & (1 << (flag)))
>>  
> 
> I wish there was also an helper to set these bits. it gives me an heart attack
> every time I need to:
> 	bio->bi_rw &= ~(1 << BIO_RW);

What's more disturbing to me is the different between RQ and BIO
flags.  __REQ_* are bit positions, REQ_* are masks while BIO_* are bit
positions.  Sadly it seems it's already too late to change that.  I
personally an not a big fan of simple accessors or flags defined as
bit positions.  They seem to obscure things without much benefit.

Thanks.

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