From patchwork Sat Dec 22 22:30:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Kravetz X-Patchwork-Id: 10741527 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 053766C5 for ; Sat, 22 Dec 2018 22:30:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E82CB28A5D for ; Sat, 22 Dec 2018 22:30:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DC7F128A60; Sat, 22 Dec 2018 22:30:51 +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=-3.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8B51828A5D for ; Sat, 22 Dec 2018 22:30:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5149A8E0004; Sat, 22 Dec 2018 17:30:46 -0500 (EST) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id 4C94D8E0001; Sat, 22 Dec 2018 17:30:46 -0500 (EST) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2F13E8E0004; Sat, 22 Dec 2018 17:30:46 -0500 (EST) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from mail-it1-f200.google.com (mail-it1-f200.google.com [209.85.166.200]) by kanga.kvack.org (Postfix) with ESMTP id F22888E0001 for ; Sat, 22 Dec 2018 17:30:45 -0500 (EST) Received: by mail-it1-f200.google.com with SMTP id w15so9533712ita.1 for ; Sat, 22 Dec 2018 14:30:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:dkim-signature:from:to:cc:subject:date :message-id:in-reply-to:references; bh=VKohsUrMKKgNGnQ0gYAjxcXhQ+gP8dLYA/uers/DyHM=; b=dOpjKVNRmhCt4QFPWALN807r4cFngFhL7tkU949d4fodsEYh+kXoVLHN1rrPxacUvp hoUG+AqHs1KQ0wvzXIKLG3wJqobViimCd/QmpT8vJIREJjIpm/akCwA6sbdOVbuyiZyf fA/cXqZfskTp31iAKmtCAvcIaEVjSM/1bvmJlwVJmGVSIoFoX4noBdqhiVAPIgUfihpj VYf8zViOVohiBZIYBKpInm+eQO0EmRj3y3fa8F2Pa6yd6XgJo5VGZo1LBDC8iOYWd25V 3JMGZkdeu55MNx5vQrZEoPH0SjJzgCfFEpaBfW8Q01Qi11YNiHWXW3cuMlEa2yqTX4X0 ujJg== X-Gm-Message-State: AA+aEWaJn10/BoPi23Vl4Mr07uNr4h0nNZlE7ISsUZUs+Bwu1W+VepZj fdoyfeugGUiyO32snvPH22s/ym8RjJfLpHAr6FjaRm03iRMsLB6yonNymi7LZBrPF+nJpynDMOe tehngiDzHGiazXstyUHYIJguQLQ1tfaKQ/tSJAF/rZQ2DNClh3FVZhivsUijJ7o+AHA== X-Received: by 2002:a24:2452:: with SMTP id f79mr5808054ita.143.1545517845409; Sat, 22 Dec 2018 14:30:45 -0800 (PST) X-Google-Smtp-Source: AFSGD/U/P/J3YbUiYpkaWmY5zK/Nt65mgCCK8EEpsoZDzZhNZHIU2syhngIx3pQvvSIHT/YDhlv9 X-Received: by 2002:a24:2452:: with SMTP id f79mr5808021ita.143.1545517844184; Sat, 22 Dec 2018 14:30:44 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1545517844; cv=none; d=google.com; s=arc-20160816; b=IaTK0qHnTDGi2C9IlP7SWpg6r5+0zgLF1kvCLXbVf/C1xW2XZH9k0fpEaWuNVzPeNC K40TxFaOn9V5ptqEpBScoPFyTgaedNSSa9G586J+64zBIOZ9Cd4zSp3PL2RbJLJOYHga AjYIFZi1dZuQyKxjL44yC7O4tZ7rUxcciQ/7GYBQBWwYNJhvAAL8ZCMI3hINGnJGJoQ0 ms83nxW0wKJKRvBehKvdY/S5aGbK3jbFDE3i8UBsuynPPXCfC5NBYkihbPgIheSI75IE ATPHZQaiA99nqY6tp7zGS4mpKabzHhEZWwQlBSju74PdmaZ08y7YJF1hBQm0wUuSeRY2 KB/g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=VKohsUrMKKgNGnQ0gYAjxcXhQ+gP8dLYA/uers/DyHM=; b=uUw0ROa3LsRzTYG07e3n0NaliD8LMwqEj7T8OCIsX54sjyyS84v0AGidNLseaCa9R2 LDoVnizHaQeum/NJiKPke6raG5OD3zb5ThBzU7N2EEFB8tWXxt7e0x6aPdLRSrGNCRs+ NUac6QUi5vTx7XxtsAReh3g8JDs6UIWiZyXOdj6bPnrs+yHyrdwEwOYFjKwXwjbzYcm2 l+a88E8sVXdy5AUDQFDhnNKojKn1SQBZc8mU7FSoOTzDZJzsuSpF0aKIvuC6h0vVU594 c/zqbBJQbUHGmgS39igOdIIBKtjyF7LhGK4M87Sd2HCYxYoozDENYiGBwh/LO47GDJUL Wk4Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2018-07-02 header.b=cawiLHyv; spf=pass (google.com: domain of mike.kravetz@oracle.com designates 141.146.126.79 as permitted sender) smtp.mailfrom=mike.kravetz@oracle.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Received: from aserp2130.oracle.com (aserp2130.oracle.com. [141.146.126.79]) by mx.google.com with ESMTPS id b21si7593350itc.73.2018.12.22.14.30.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 22 Dec 2018 14:30:44 -0800 (PST) Received-SPF: pass (google.com: domain of mike.kravetz@oracle.com designates 141.146.126.79 as permitted sender) client-ip=141.146.126.79; Authentication-Results: mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2018-07-02 header.b=cawiLHyv; spf=pass (google.com: domain of mike.kravetz@oracle.com designates 141.146.126.79 as permitted sender) smtp.mailfrom=mike.kravetz@oracle.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id wBMMTqYX066326; Sat, 22 Dec 2018 22:30:37 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : date : message-id : in-reply-to : references; s=corp-2018-07-02; bh=VKohsUrMKKgNGnQ0gYAjxcXhQ+gP8dLYA/uers/DyHM=; b=cawiLHyvjX8YZrNblKRgGF2vBdj3B//f2HVRxrhsu+XtAz5UM2lbR8HXrLiY9/BaZb2a 2n9L02+IHeEE+rezbMDJE/xNfPrXGNdmeFArjT7OCDph0ZoPBLjyKIuU4mTwqjaiJof6 xIs3kVf4ZEUaedyCPoe2hQJYSLEuCvVKOCxcz5jINxTt8oEHXfwdpD2hk/jNM4qTxsDE pg1TJUb4qffz9hfjapPG7E/aLoIkiJR5XLnESiH9aRHxpS+FJWPIkEjnllkU72IrfgU+ Ri7n4LDOcVk6jNhAOs49zCgTR80spaf2n4msAzmxM+0lQwhcUIei1zk+U7gbXjfQKg/W lA== Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by aserp2130.oracle.com with ESMTP id 2phasdhh0p-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sat, 22 Dec 2018 22:30:37 +0000 Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id wBMMUadv012434 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sat, 22 Dec 2018 22:30:36 GMT Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id wBMMUZZw013491; Sat, 22 Dec 2018 22:30:35 GMT Received: from monkey.oracle.com (/50.38.38.67) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sat, 22 Dec 2018 14:30:35 -0800 From: Mike Kravetz To: linux-mm@kvack.org, linux-kernel@vger.kernel.org Cc: Michal Hocko , Hugh Dickins , Naoya Horiguchi , "Aneesh Kumar K . V" , Andrea Arcangeli , "Kirill A . Shutemov" , Davidlohr Bueso , Prakash Sangappa , Andrew Morton , Mike Kravetz , stable@vger.kernel.org Subject: [PATCH v3 2/2] hugetlbfs: Use i_mmap_rwsem to fix page fault/truncate race Date: Sat, 22 Dec 2018 14:30:13 -0800 Message-Id: <20181222223013.22193-3-mike.kravetz@oracle.com> X-Mailer: git-send-email 2.17.2 In-Reply-To: <20181222223013.22193-1-mike.kravetz@oracle.com> References: <20181222223013.22193-1-mike.kravetz@oracle.com> X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=9115 signatures=668680 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=972 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1812220204 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: X-Virus-Scanned: ClamAV using ClamSMTP hugetlbfs page faults can race with truncate and hole punch operations. Current code in the page fault path attempts to handle this by 'backing out' operations if we encounter the race. One obvious omission in the current code is removing a page newly added to the page cache. This is pretty straight forward to address, but there is a more subtle and difficult issue of backing out hugetlb reservations. To handle this correctly, the 'reservation state' before page allocation needs to be noted so that it can be properly backed out. There are four distinct possibilities for reservation state: shared/reserved, shared/no-resv, private/reserved and private/no-resv. Backing out a reservation may require memory allocation which could fail so that needs to be taken into account as well. Instead of writing the required complicated code for this rare occurrence, just eliminate the race. i_mmap_rwsem is now held in read mode for the duration of page fault processing. Hold i_mmap_rwsem longer in truncation and hold punch code to cover the call to remove_inode_hugepages. With this modification, code in remove_inode_hugepages checking for races becomes 'dead' as it can not longer happen. Remove the dead code and expand comments to explain reasoning. Similarly, checks for races with truncation in the page fault path can be simplified and removed. Cc: Fixes: ebed4bfc8da8 ("hugetlb: fix absurd HugePages_Rsvd") Signed-off-by: Mike Kravetz --- fs/hugetlbfs/inode.c | 61 ++++++++++++++++++++------------------------ mm/hugetlb.c | 21 ++++++++------- 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index 32920a10100e..a2fcea5f8225 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -383,17 +383,16 @@ hugetlb_vmdelete_list(struct rb_root_cached *root, pgoff_t start, pgoff_t end) * truncation is indicated by end of range being LLONG_MAX * In this case, we first scan the range and release found pages. * After releasing pages, hugetlb_unreserve_pages cleans up region/reserv - * maps and global counts. Page faults can not race with truncation - * in this routine. hugetlb_no_page() prevents page faults in the - * truncated range. It checks i_size before allocation, and again after - * with the page table lock for the page held. The same lock must be - * acquired to unmap a page. + * maps and global counts. * hole punch is indicated if end is not LLONG_MAX * In the hole punch case we scan the range and release found pages. * Only when releasing a page is the associated region/reserv map * deleted. The region/reserv map for ranges without associated - * pages are not modified. Page faults can race with hole punch. - * This is indicated if we find a mapped page. + * pages are not modified. + * + * Callers of this routine must hold the i_mmap_rwsem in write mode to prevent + * races with page faults. + * * Note: If the passed end of range value is beyond the end of file, but * not LLONG_MAX this routine still performs a hole punch operation. */ @@ -423,32 +422,14 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart, for (i = 0; i < pagevec_count(&pvec); ++i) { struct page *page = pvec.pages[i]; - u32 hash; index = page->index; - hash = hugetlb_fault_mutex_hash(h, current->mm, - &pseudo_vma, - mapping, index, 0); - mutex_lock(&hugetlb_fault_mutex_table[hash]); - /* - * If page is mapped, it was faulted in after being - * unmapped in caller. Unmap (again) now after taking - * the fault mutex. The mutex will prevent faults - * until we finish removing the page. - * - * This race can only happen in the hole punch case. - * Getting here in a truncate operation is a bug. + * A mapped page is impossible as callers should unmap + * all references before calling. And, i_mmap_rwsem + * prevents the creation of additional mappings. */ - if (unlikely(page_mapped(page))) { - BUG_ON(truncate_op); - - i_mmap_lock_write(mapping); - hugetlb_vmdelete_list(&mapping->i_mmap, - index * pages_per_huge_page(h), - (index + 1) * pages_per_huge_page(h)); - i_mmap_unlock_write(mapping); - } + VM_BUG_ON(page_mapped(page)); lock_page(page); /* @@ -470,7 +451,6 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart, } unlock_page(page); - mutex_unlock(&hugetlb_fault_mutex_table[hash]); } huge_pagevec_release(&pvec); cond_resched(); @@ -482,9 +462,20 @@ static void remove_inode_hugepages(struct inode *inode, loff_t lstart, static void hugetlbfs_evict_inode(struct inode *inode) { + struct address_space *mapping = inode->i_mapping; struct resv_map *resv_map; + /* + * The vfs layer guarantees that there are no other users of this + * inode. Therefore, it would be safe to call remove_inode_hugepages + * without holding i_mmap_rwsem. We acquire and hold here to be + * consistent with other callers. Since there will be no contention + * on the semaphore, overhead is negligible. + */ + i_mmap_lock_write(mapping); remove_inode_hugepages(inode, 0, LLONG_MAX); + i_mmap_unlock_write(mapping); + resv_map = (struct resv_map *)inode->i_mapping->private_data; /* root inode doesn't have the resv_map, so we should check it */ if (resv_map) @@ -505,8 +496,8 @@ static int hugetlb_vmtruncate(struct inode *inode, loff_t offset) i_mmap_lock_write(mapping); if (!RB_EMPTY_ROOT(&mapping->i_mmap.rb_root)) hugetlb_vmdelete_list(&mapping->i_mmap, pgoff, 0); - i_mmap_unlock_write(mapping); remove_inode_hugepages(inode, offset, LLONG_MAX); + i_mmap_unlock_write(mapping); return 0; } @@ -540,8 +531,8 @@ static long hugetlbfs_punch_hole(struct inode *inode, loff_t offset, loff_t len) hugetlb_vmdelete_list(&mapping->i_mmap, hole_start >> PAGE_SHIFT, hole_end >> PAGE_SHIFT); - i_mmap_unlock_write(mapping); remove_inode_hugepages(inode, hole_start, hole_end); + i_mmap_unlock_write(mapping); inode_unlock(inode); } @@ -624,7 +615,11 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset, /* addr is the offset within the file (zero based) */ addr = index * hpage_size; - /* mutex taken here, fault path and hole punch */ + /* + * fault mutex taken here, protects against fault path + * and hole punch. inode_lock previously taken protects + * against truncation. + */ hash = hugetlb_fault_mutex_hash(h, mm, &pseudo_vma, mapping, index, addr); mutex_lock(&hugetlb_fault_mutex_table[hash]); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 2a3162030167..cfd9790b01e3 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3757,16 +3757,16 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm, } /* - * Use page lock to guard against racing truncation - * before we get page_table_lock. + * We can not race with truncation due to holding i_mmap_rwsem. + * Check once here for faults beyond end of file. */ + size = i_size_read(mapping->host) >> huge_page_shift(h); + if (idx >= size) + goto out; + retry: page = find_lock_page(mapping, idx); if (!page) { - size = i_size_read(mapping->host) >> huge_page_shift(h); - if (idx >= size) - goto out; - /* * Check for page in userfault range */ @@ -3856,9 +3856,6 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm, } ptl = huge_pte_lock(h, mm, ptep); - size = i_size_read(mapping->host) >> huge_page_shift(h); - if (idx >= size) - goto backout; ret = 0; if (!huge_pte_none(huge_ptep_get(ptep))) @@ -3961,8 +3958,10 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, /* * Acquire i_mmap_rwsem before calling huge_pte_alloc and hold - * until finished with ptep. This prevents huge_pmd_unshare from - * being called elsewhere and making the ptep no longer valid. + * until finished with ptep. This serves two purposes: + * 1) It prevents huge_pmd_unshare from being called elsewhere + * and making the ptep no longer valid. + * 2) It synchronizes us with file truncation. * * ptep could have already be assigned via huge_pte_offset. That * is OK, as huge_pte_alloc will return the same value unless