linux-kernel - Re: [PATCH] block: optimise bvec_iter

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 30 Nov 2019 01:47:16 +0300
From:   Pavel Begunkov <asml.silence@...il.com>
To:     Arvind Sankar <nivedita@...m.mit.edu>
Cc:     Jens Axboe <axboe@...nel.dk>, Ming Lei <ming.lei@...onical.com>,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] block: optimise bvec_iter_advance()

On 30/11/2019 01:17, Arvind Sankar wrote:
> 
> The loop can be simplified a bit further, as done has to be 0 once we go
> beyond the current bio_vec. See below for the simplified version.
> 

Thanks for the suggestion! I thought about it, and decided to not
for several reasons. I prefer to not fine-tune and give compilers
more opportunity to do their job. And it's already fast enough with
modern architectures (MOVcc, complex addressing, etc).

Also need to consider code clarity and the fact, that this is inline,
so should be brief and register-friendly.


> I also check if bi_size became zero so we can skip the rest of the
> calculations in that case. If we want to preserve the current behavior of
> updating iter->bi_idx and iter->bi_bvec_done even if bi_size is going to
> become zero, the loop condition should change to
> 
> 		while (bytes && bytes >= cur->bv_len)

Probably, it's better to leave it in a consistent state. Shouldn't be
a problem, but never know when and who will screw it up. 

> 
> to ensure that we don't try to access past the end of the bio_vec array.
> 
> diff --git a/include/linux/bvec.h b/include/linux/bvec.h
> index a032f01e928c..380d188dfbd2 100644
> --- a/include/linux/bvec.h
> +++ b/include/linux/bvec.h
> @@ -87,25 +87,26 @@ struct bvec_iter_all {
>  static inline bool bvec_iter_advance(const struct bio_vec *bv,
>  		struct bvec_iter *iter, unsigned bytes)
>  {
> +	unsigned int idx = iter->bi_idx;
> +	const struct bio_vec *cur = bv + idx;
> +
>  	if (WARN_ONCE(bytes > iter->bi_size,
>  		     "Attempted to advance past end of bvec iter\n")) {
>  		iter->bi_size = 0;
>  		return false;
>  	}
>  
> -	while (bytes) {
> -		const struct bio_vec *cur = bv + iter->bi_idx;
> -		unsigned len = min3(bytes, iter->bi_size,
> -				    cur->bv_len - iter->bi_bvec_done);
> -
> -		bytes -= len;
> -		iter->bi_size -= len;
> -		iter->bi_bvec_done += len;
> -
> -		if (iter->bi_bvec_done == cur->bv_len) {
> -			iter->bi_bvec_done = 0;
> -			iter->bi_idx++;
> +	iter->bi_size -= bytes;
> +	if (iter->bi_size != 0) {
> +		bytes += iter->bi_bvec_done;
> +		while (bytes >= cur->bv_len) {
> +			bytes -= cur->bv_len;
> +			idx++;
> +			cur++;
>  		}
> +
> +		iter->bi_idx = idx;
> +		iter->bi_bvec_done = bytes;
>  	}
>  	return true;
>  }
> 

-- 
Pavel Begunkov



Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)