From patchwork Fri Feb 10 17:24:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Wilcox X-Patchwork-Id: 9567197 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5117D601C3 for ; Fri, 10 Feb 2017 17:46:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 418FB285AD for ; Fri, 10 Feb 2017 17:46:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 35BE0285B0; Fri, 10 Feb 2017 17:46:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 95A29285AD for ; Fri, 10 Feb 2017 17:46:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751726AbdBJRqL (ORCPT ); Fri, 10 Feb 2017 12:46:11 -0500 Received: from bombadil.infradead.org ([65.50.211.133]:53739 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752024AbdBJRqL (ORCPT ); Fri, 10 Feb 2017 12:46:11 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=KVMacDg07yegXltX09nyMev0/OZAntGPs5vlQb3lr+k=; b=SC5C0fA0BGun2rtv9UfswfrXl PtHuqsvaD7dnnOnL1yHjLLVTWq1Les/EQrBoaCjnn8vnuNJol7dTcJk+sAUz1rqI35D5lrON1woGd PHJWQMCiOLyZ/zT7ZQ5yMpgIq5D55bqreXBy3c8mFf97GYPkBwsmNBtGeL8zHy6tbyV32yr9mef9i +nB2yXRV5lCJqDkfyBUHBD5SI/zFHshLhbLLSvQr5Fz1oqeRx2eqiLayo9jG/2o1RzQtC6n81KpXd 3mxdNdfc7DJ+m6fFO9MSu3O0sKq3PSc6x+KJs2tQOMC5Leh6c2I+YdvJtJe59UAA7E022NtdxfE43 3a3Oa4hUw==; Received: from willy by bombadil.infradead.org with local (Exim 4.87 #1 (Red Hat Linux)) id 1ccEvR-00044M-Kw; Fri, 10 Feb 2017 17:24:01 +0000 Date: Fri, 10 Feb 2017 09:24:01 -0800 From: Matthew Wilcox To: "Kirill A. Shutemov" Cc: Theodore Ts'o , Andreas Dilger , Jan Kara , Andrew Morton , Alexander Viro , Hugh Dickins , Andrea Arcangeli , Dave Hansen , Vlastimil Babka , Ross Zwisler , linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-block@vger.kernel.org Subject: Re: [PATCHv6 12/37] brd: make it handle huge pages Message-ID: <20170210172401.GB2267@bombadil.infradead.org> References: <20170126115819.58875-1-kirill.shutemov@linux.intel.com> <20170126115819.58875-13-kirill.shutemov@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170126115819.58875-13-kirill.shutemov@linux.intel.com> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jan 26, 2017 at 02:57:54PM +0300, Kirill A. Shutemov wrote: > Do not assume length of bio segment is never larger than PAGE_SIZE. > With huge pages it's HPAGE_PMD_SIZE (2M on x86-64). I don't think we even need hugepages for BRD to be buggy. I think there are already places which allocate compound pages (not in highmem, obviously ...) and put them in biovecs. So this is pure and simple a bugfix. That said, I find the current code in brd a bit inelegant, and I don't think this patch helps... indeed, I think it's buggy: > @@ -202,12 +202,15 @@ static int copy_to_brd_setup(struct brd_device *brd, sector_t sector, size_t n) > size_t copy; > > copy = min_t(size_t, n, PAGE_SIZE - offset); > + n -= copy; > if (!brd_insert_page(brd, sector)) > return -ENOSPC; > - if (copy < n) { > + while (n) { > sector += copy >> SECTOR_SHIFT; > if (!brd_insert_page(brd, sector)) > return -ENOSPC; > + copy = min_t(size_t, n, PAGE_SIZE); > + n -= copy; > } We're decrementing 'n' to 0, then testing it, so we never fill in the last page ... right? Anyway, here's my effort. Untested. diff --git a/drivers/block/brd.c b/drivers/block/brd.c index 3adc32a3153b..0802a6abcd81 100644 --- a/drivers/block/brd.c +++ b/drivers/block/brd.c @@ -202,12 +202,14 @@ static int copy_to_brd_setup(struct brd_device *brd, sector_t sector, size_t n) size_t copy; copy = min_t(size_t, n, PAGE_SIZE - offset); - if (!brd_insert_page(brd, sector)) - return -ENOSPC; - if (copy < n) { - sector += copy >> SECTOR_SHIFT; + for (;;) { if (!brd_insert_page(brd, sector)) return -ENOSPC; + n -= copy; + if (!n) + break; + sector += copy >> SECTOR_SHIFT; + copy = min_t(size_t, n, PAGE_SIZE); } return 0; } @@ -239,26 +241,23 @@ static void copy_to_brd(struct brd_device *brd, const void *src, struct page *page; void *dst; unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT; - size_t copy; + size_t copy = min_t(size_t, n, PAGE_SIZE - offset); - copy = min_t(size_t, n, PAGE_SIZE - offset); - page = brd_lookup_page(brd, sector); - BUG_ON(!page); - - dst = kmap_atomic(page); - memcpy(dst + offset, src, copy); - kunmap_atomic(dst); - - if (copy < n) { - src += copy; - sector += copy >> SECTOR_SHIFT; - copy = n - copy; + for (;;) { page = brd_lookup_page(brd, sector); BUG_ON(!page); dst = kmap_atomic(page); - memcpy(dst, src, copy); + memcpy(dst + offset, src, copy); kunmap_atomic(dst); + + n -= copy; + if (!n) + break; + src += copy; + sector += copy >> SECTOR_SHIFT; + offset = 0; + copy = min_t(size_t, n, PAGE_SIZE); } } @@ -271,28 +270,24 @@ static void copy_from_brd(void *dst, struct brd_device *brd, struct page *page; void *src; unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT; - size_t copy; + size_t copy = min_t(size_t, n, PAGE_SIZE - offset); - copy = min_t(size_t, n, PAGE_SIZE - offset); - page = brd_lookup_page(brd, sector); - if (page) { - src = kmap_atomic(page); - memcpy(dst, src + offset, copy); - kunmap_atomic(src); - } else - memset(dst, 0, copy); - - if (copy < n) { - dst += copy; - sector += copy >> SECTOR_SHIFT; - copy = n - copy; + for (;;) { page = brd_lookup_page(brd, sector); if (page) { src = kmap_atomic(page); - memcpy(dst, src, copy); + memcpy(dst, src + offset, copy); kunmap_atomic(src); } else memset(dst, 0, copy); + + n -= copy; + if (!n) + break; + dst += copy; + sector += copy >> SECTOR_SHIFT; + offset = 0; + copy = min_t(size_t, n, PAGE_SIZE); } }