linux-kernel - Re: [PATCH 13/17] blk-map: implement blk_rq_map_kern

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: <49D3E9EF.8010407@kernel.org>
Date:	Thu, 02 Apr 2009 07:25:51 +0900
From:	Tejun Heo <tj@...nel.org>
To:	Boaz Harrosh <bharrosh@...asas.com>
CC:	axboe@...nel.dk, linux-kernel@...r.kernel.org,
	fujita.tomonori@....ntt.co.jp
Subject: Re: [PATCH 13/17] blk-map: implement blk_rq_map_kern_sgl()

Hello,

Boaz Harrosh wrote:
>> -	do_copy = !blk_rq_aligned(q, kbuf, len) || object_is_on_stack(kbuf);
>> -	if (do_copy)
>> -		bio = bio_copy_kern(q, kbuf, len, gfp, rw);
>> -	else
>> -		bio = bio_map_kern(q, kbuf, len, gfp);
>> -
>> +	bio = bio_map_kern_sg(q, sgl, nents, rw, gfp);
>> +	if (IS_ERR(bio))
>> +		bio = bio_copy_kern_sg(q, sgl, nents, rw, gfp);
>>  	if (IS_ERR(bio))
>>  		return PTR_ERR(bio);
> 
> You might want to call bio_copy_kern_sg from within bio_map_kern_sg
> and remove yet another export from bio layer
> 
> Same thing with bio_map_user_iov/bio_copy_user_iov

Right, that makes sense.  Will incorporate it.

>>  
>> -	if (rq_data_dir(rq) == WRITE)
>> -		bio->bi_rw |= (1 << BIO_RW);
>> -
>> -	if (do_copy)
>> +	if (!bio_flagged(bio, BIO_USER_MAPPED))
>>  		rq->cmd_flags |= REQ_COPY_USER;
>>  
>> +	blk_queue_bounce(q, &bio);
>>  	blk_rq_bio_prep(q, rq, bio);
> 
> It could be nice to call blk_rq_append_bio() here
> and support multiple calls to this member.
> This will solve half of my problem with osd_initiator
> 
> Hmm .. but you wanted to drop multiple bio chaining
> perhaps you would reconsider.

I don't want to drop multiple bio chaining at all in itself.  I just
didn't see the current uses as, well, useful.  If building a sgl for a
request at once is difficult for your purpose, making blk_rq_map_*()
functions accumulate bios sounds like a good idea.  The primary goal
was to remove direct bio visibility/manipulation from low level
driver's POV.

>> +int blk_rq_map_kern(struct request_queue *q, struct request *rq, void *kbuf,
>> +		    unsigned int len, gfp_t gfp)
>> +{
>> +	struct scatterlist sg;
>> +
>> +	sg_init_one(&sg, kbuf, len);
>> +
>> +	return blk_rq_map_kern_sg(q, rq, &sg, 1, gfp);
>> +}
>>  EXPORT_SYMBOL(blk_rq_map_kern);
> 
> could be inline like with the end_request functions

Yeap, will make it inline.

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/