From patchwork Wed Mar 16 22:38:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Busch X-Patchwork-Id: 8605871 Return-Path: X-Original-To: patchwork-linux-block@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A720A9F758 for ; Wed, 16 Mar 2016 22:38:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D53BC20120 for ; Wed, 16 Mar 2016 22:38:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DE71520218 for ; Wed, 16 Mar 2016 22:38:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932684AbcCPWiJ (ORCPT ); Wed, 16 Mar 2016 18:38:09 -0400 Received: from mga03.intel.com ([134.134.136.65]:30065 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932546AbcCPWiG (ORCPT ); Wed, 16 Mar 2016 18:38:06 -0400 Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP; 16 Mar 2016 15:38:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,346,1455004800"; d="scan'208";a="67832953" Received: from localhost.lm.intel.com ([10.232.112.36]) by fmsmga004.fm.intel.com with ESMTP; 16 Mar 2016 15:38:04 -0700 Date: Wed, 16 Mar 2016 22:38:04 +0000 From: Keith Busch To: Vitaly Kuznetsov Cc: Ming Lei , linux-block@vger.kernel.org, Linux Kernel Mailing List , Jens Axboe , Dan Williams , "Martin K. Petersen" , Sagi Grimberg , Mike Snitzer , "K. Y. Srinivasan" , Cathy Avery Subject: Re: [PATCH RFC] block: fix bio merge checks when virt_boundary is set Message-ID: <20160316223804.GA6217@localhost.lm.intel.com> References: <1458055076-2175-1-git-send-email-vkuznets@redhat.com> <87oaae4cej.fsf@vitty.brq.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87oaae4cej.fsf@vitty.brq.redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Mar 16, 2016 at 05:26:28PM +0100, Vitaly Kuznetsov wrote: > Ming Lei writes: > > We do have the above merge in bio_add_page(), so the two bios in > > your above example shouldn't have been observed if the two buffers > > are added to bio via the bio_add_page(). > > > > If you see short bios in above example, maybe you need to check ntfs code: > > > > - if bio_add_page() is used to add buffer > > - if using one standalone bio to transfer each 512byte, even they > > are in same page and the sector is continuous > > I'm not using ntfs, mkfs.ntfs is a userspace application which shows the > regression when virt_boundary is in place. I should have avoided > mentioning bio_add_pc_page() here as it is unrelated to the issue. > > In particular, I'm concearned about the following call sites: > blk_bio_segment_split() > ll_back_merge_fn() > ll_front_merge_fn() I don't think blk_bio_segment_split would have seen such a bio vector if it pages were added with bio_add_page. Those should already have been combined. In any case, I think you can get what you're after just by moving the gap check after BIOVEC_PHYS_MERGABLE. Does the following look ok to you? --- -- -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/block/blk-merge.c b/block/blk-merge.c index 2613531..4aa8e44 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -96,13 +96,6 @@ static struct bio *blk_bio_segment_split(struct request_queue *q, const unsigned max_sectors = get_max_io_size(q, bio); bio_for_each_segment(bv, bio, iter) { - /* - * If the queue doesn't support SG gaps and adding this - * offset would create a gap, disallow it. - */ - if (bvprvp && bvec_gap_to_prev(q, bvprvp, bv.bv_offset)) - goto split; - if (sectors + (bv.bv_len >> 9) > max_sectors) { /* * Consider this a new segment if we're splitting in @@ -139,6 +132,13 @@ new_segment: if (nsegs == queue_max_segments(q)) goto split; + /* + * If the queue doesn't support SG gaps and adding this + * offset would create a gap, disallow it. + */ + if (bvprvp && bvec_gap_to_prev(q, bvprvp, bv.bv_offset)) + goto split; + nsegs++; bvprv = bv; bvprvp = &bvprv; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 413c84f..69cffbe 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1400,7 +1400,8 @@ static inline bool bio_will_gap(struct request_queue *q, struct bio *prev, bio_get_last_bvec(prev, &pb); bio_get_first_bvec(next, &nb); - return __bvec_gap_to_prev(q, &pb, nb.bv_offset); + if (!BIOVEC_PHYS_MERGEABLE(&pb, &nb)) + return __bvec_gap_to_prev(q, &pb, nb.bv_offset); } return false;