linux-kernel - Re: [PATCH block#for-2.6.31] block: add request clone interface

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: <20090610043034.GW11363@kernel.dk>
Date:	Wed, 10 Jun 2009 06:30:34 +0200
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Kiyoshi Ueda <k-ueda@...jp.nec.com>
Cc:	linux-kernel@...r.kernel.org,
	device-mapper development <dm-devel@...hat.com>,
	Jun'ichi Nomura <j-nomura@...jp.nec.com>,
	Boaz Harrosh <bharrosh@...asas.com>
Subject: Re: [PATCH block#for-2.6.31] block: add request clone interface

On Wed, Jun 10 2009, Kiyoshi Ueda wrote:
> Hi Jens,
> 
> On 06/10/2009 03:03 AM +0900, Jens Axboe wrote:
> > On Tue, Jun 09 2009, Kiyoshi Ueda wrote:
> >> Hi Jens,
> >>
> >> +/*
> >> + * Copy request information of the original request to the clone request.
> >> + */
> >> +static void __blk_rq_prep_clone(struct request *dst, struct request *src)
> >> +{
> >> +	dst->cpu = src->cpu;
> >> +	dst->cmd_flags = (rq_data_dir(src) | REQ_NOMERGE);
> >> +	dst->cmd_type = src->cmd_type;
> >> +	dst->__sector = blk_rq_pos(src);
> >> +	dst->__data_len = blk_rq_bytes(src);
> >> +	dst->nr_phys_segments = src->nr_phys_segments;
> >> +	dst->ioprio = src->ioprio;
> >> +	dst->buffer = src->buffer;
> >> +	dst->cmd_len = src->cmd_len;
> >> +	dst->cmd = src->cmd;
> > 
> > Are you making sure that 'src' always exists while 'dst' is alive?
> 
> Yes.
> Request-based dm is the owner of 'src' (original) and
> it never frees 'src' until the 'dst' (clone) are completed.
> 
> I avoided deep-copying __cmd/buffer/sense as it's costly
> (additional allocation and memcpy).
> And I don't think there are any needs for that.
> But if anyone really wants that even with the copying cost,
> please speak up.

I just worry that the interface is easy to misuse. You don't document
the requirement that the src request may not go away while dst is used,
yet it's an important fact. The function advertises itself as a copy,
you would not normally expect any such restrictions.

-- 
Jens Axboe

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