From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boaz Harrosh Subject: Re: [PATCH block#for-2.6.31] block: add request clone interface Date: Wed, 10 Jun 2009 10:55:25 +0300 Message-ID: <4A2F66ED.80206@panasas.com> References: <4A2E1490.7060902@ct.jp.nec.com> <20090609180344.GQ11363@kernel.dk> <4A2F1741.8090100@ct.jp.nec.com> Reply-To: device-mapper development Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4A2F1741.8090100@ct.jp.nec.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Kiyoshi Ueda , Tejun Heo Cc: Jun'ichi Nomura , device-mapper development , linux-kernel@vger.kernel.org, Jens Axboe List-Id: dm-devel.ids On 06/10/2009 05:15 AM, 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). For my needs for example dst->cmd will be different then src->cmd. Could be untouched. The caller will set what he needs. dst->sense should be untouched, caller can set to src->sense if he wants to. Or like me he already have another buffer. dst->buffer is always NULL in my path so I don't know what that is. Tejun? It should only be about bios and lengths. And a big fat comment about what it does and what it does not. > 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. > > Thanks, > Kiyoshi Ueda Thanks Boaz