From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751707AbdIUCs5 (ORCPT ); Wed, 20 Sep 2017 22:48:57 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:25272 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751387AbdIUCs4 (ORCPT ); Wed, 20 Sep 2017 22:48:56 -0400 Subject: Re: [PATCH] block: consider merge of segments when merge bio into rq To: Christoph Hellwig Cc: Jens Axboe , linux-block@vger.kernel.org, linux-kernel@vger.kernel.org References: <1505517709-2478-1-git-send-email-jianchao.w.wang@oracle.com> <20170921012949.GA29873@infradead.org> From: "jianchao.wang" Message-ID: Date: Thu, 21 Sep 2017 10:46:23 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20170921012949.GA29873@infradead.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/21/2017 09:29 AM, Christoph Hellwig wrote: > So the check change here looks good to me. > > I don't like like the duplicate code, can you look into sharing > the new segment checks between the two functions and the existing > instance in ll_merge_requests_fn by passing say two struct bio *bio1 > and struct bio *bio2 pointer instead of using req->bio and req->biotail? > > Also please include the information that you posted in the reply to > the other patch in the description for this one. > I have looked into the ll_back/front_merge_fn() and ll_merge_requests_fn(). It seems that segments check code fragment looks similar but not unified. We could unify the part of calculation of variable of seg_size and contig, but not the positions where use these two. Especially the calculation of total_phys_segments and judgment of 'nr_phys_segments == 1', rq variable cannot be avoided there. And the update of bi_seg_front/back_size is also very tricky. If force to merge these code fragments together, the code may become hard to read. So please refer to the V2 patch which contain more comment about the issue and result. Thanks Jianchao