From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BAB75C77B75 for ; Mon, 22 May 2023 20:56:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 05A656B0074; Mon, 22 May 2023 16:56:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 00A296B0075; Mon, 22 May 2023 16:56:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E154F900002; Mon, 22 May 2023 16:56:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id CE8646B0074 for ; Mon, 22 May 2023 16:56:19 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 7EFF240494 for ; Mon, 22 May 2023 20:56:19 +0000 (UTC) X-FDA: 80819098878.08.E4C9766 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf24.hostedemail.com (Postfix) with ESMTP id BAA0D18001A for ; Mon, 22 May 2023 20:56:17 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=Lrcvsrg3; dmarc=none; spf=pass (imf24.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684788977; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=peWBofikaK9krG5qD+2iLb6MYI9idMoJ+NdOi3ysD9Y=; b=NZxDYRX5qSJ0wGMb/NohWYk99LCSA9iqdHm3GKxaimJCzB9CFBC2QhDqXs6HeF2ECvHmpN VbSoqYJZx1bT+JdDtkF7etu8VkJUO4xND2gTW4FYh0y+wImJ3BMzAKhDyd3EC9qzouDYoZ LMH/NdVUaMuqKI4fJcaYUxaxuVsDA8A= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=Lrcvsrg3; dmarc=none; spf=pass (imf24.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684788977; a=rsa-sha256; cv=none; b=YQE4cDgEoycCAsA+NTsh8cvQjbH53d/TSV1/EmffYJaBSEN6pSQvN0/vjubCdI1ObYeZS0 Or6KmuWm3C2U3vqq/fKKsNzQxixMPkTe+fF6Q3Bu2UO5axSkqgPgmuGwpM++VUG4azltaK VFJalK+Jw13KL09loipxdp9iTrO9Qhc= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CD3C161C00; Mon, 22 May 2023 20:56:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F0233C433D2; Mon, 22 May 2023 20:56:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1684788976; bh=nuY45qVrwi5Vr9irJ7j4aWh7djwvnqNlkF9mxEhuJh4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Lrcvsrg3AyMu+EmrF8C4ZBe6v6C2/Rbkxg88uemOTafccbiNGm+YgwmJcfHGrKyWC 76NT5ErfVGAWNJ5Z6Z5J1pHSWmWz70tzi0M+SVbvkTYdDpo0n+kfKZiuCn5zRuXHbU GwyU6B6D1lZDC5oRHooCEtdsw+pm49Wc9l9nyFcQ= Date: Mon, 22 May 2023 13:56:15 -0700 From: Andrew Morton To: Lorenzo Stoakes Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Mike Rapoport , "Liam R . Howlett" , Vlastimil Babka Subject: Re: [PATCH] mm/mmap: refactor mlock_future_check() Message-Id: <20230522135615.b53241a49e960281e3598898@linux-foundation.org> In-Reply-To: References: <20230522082412.56685-1-lstoakes@gmail.com> <20230522132851.ccc9fafac91d7eb9ba922e94@linux-foundation.org> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: BAA0D18001A X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: nrr8mw8cokjetsyeeje4rfm9j5dd59zr X-HE-Tag: 1684788977-366640 X-HE-Meta: U2FsdGVkX1/SToRF52mlpLA5NgioZOdMUXpKrCJCQa9+kkeQmb45oEpocXhFcZWFUskCJvBIwmE6++h+fpLHBrgK0KrOlC30XsTDPs2LdnMto+0ksX++dKe4ZkJnnu/3CbcTjWs0ocWRsdypiHF1/SV/2N1nFBvV1nSKKAJRRZLIQ+WEQ2qTI9hTEcRMFVFf2+BdHVKyIsb2cGCCP1bAcUBnqcfSddrx8w76UKZe/cFKPGHGGt90fc8Epj6uqf2tJi1KTNng6p+XxjYqYawKXTAL/pRW42Utt0q6Yf+FEPZgUjaWyqQPUPVSj6ljQMus5RL+iwE9pIMh2/8322ib2h0vm+OiWwP3SVUk2ESE8zJD5nSNyDv2P8Z00uAs7a9rhEIRnhpn/MO2om3wpRQ0bdKDcuY1+OxuUa/v98TRfgXyMfErtNHDFLrtBmAVARDjbpeyXMaQwg5XxZkuOC4ZIxXZqgV1aWIckvVm3QwQSbyrVGPxWDS7McDlRzRc80e1VnU+V8pmXfWERQnDSCmAHv0SnyLzyxLrPR07DA1XLl7b2+CneDGMtpjpCpiwQU421uUvUQCSqLNnzjtOKtByaJZ5qRkZH/cXQ9VxEq+kQVOg7vAqLShi5ougV/HkBQsf/GZZyEQBeJvdTixZFZa4PvirGoLIntZaLjbOknM4ZtiU5QzO3ITBEU5RGokS5c1esZa+1xbY1ZT+znlgRLziKg5fFxqqakI67/s6kEZpKylAbzOU7UBSWecfq6owsZIwmQag4AGA6H4NQtg5idk3pW7Q/gXY7iuzlBapgui1O04tIYq9Nbb7ta2onG9LhXze54rT04PqxPZL3ZT8fwACZnxvB/4vTR13UNcRf3WivdM3Bb23aiK9YHKaB8Y7882NngOClxmk5tD/uGNynKjd/6hDYhZMb+2PFzmefQwWMEAvLtDfqCgzQObCL7bLg1lCNl0hdNScV+Y69d/P8kl BXqDTOhC CD085yOiTeTTgLJgBPhozF+0fhRIA67kf5gycIJfD3FErWXT0hs5MS3XG9l0vB/eY+rpYxOqLT2GLuvYIgt4FZvRCnDmugGcwNlYHoPGlwhwWm5elGqa1G/HIiVJRctFTcyJcu7JSR4v17n6ldWRjjzhJvOC/Vr1D5BeLAmEzsYrfBRkv7hbdabSXoykGrnLJBirVgO7CBPVSRkaQPCAXRFKyuaDP8ARwDDdwG3XzJ+88n4iG+2bCgSa/krtb5fS823vcDCUVY7iS3N4MqUe6uHIaPRYeg4JbopPfQaGtEU60xv/Fw1s8OYeg0bqj9hdtgvQyFbR5Z+kroGtlFJvhRoyZShr5UC8boK8IJW27lPY1wtZUuoTKvcekl86pJAc2QAjpqAnmN+26gQs9mr8uM/Cns41WZxJsAiyKCWUuks6YAXT+tllYaC1g3Fn875pzVumw4LJe/iz3G1bXjMu4LSYk+uE962OL0nOoE1EzPpX3Brg359+px71CPoWscPwdl0FpJcRowTbQGB/WcbzCtEMKCw7K7fYkbLYY 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: On Mon, 22 May 2023 21:34:31 +0100 Lorenzo Stoakes wrote: > On Mon, May 22, 2023 at 01:28:51PM -0700, Andrew Morton wrote: > > On Mon, 22 May 2023 09:24:12 +0100 Lorenzo Stoakes wrote: > > > > > In all but one instance, mlock_future_check() is treated as a boolean > > > function despite returning an error code. In one instance, this error code > > > is ignored and replaced with -ENOMEM. > > > > > > This is confusing, and the inversion of true -> failure, false -> success > > > is not warranted. Convert the function to a bool, lightly refactor and > > > return true if the check passes, false if not. > > > > Yup. > > > > I don't think the name does a good job of conveying the > > function's use. > > > > > - if (mlock_future_check(mm, vm_flags, len)) > > > + if (!mlock_future_check(mm, vm_flags, len)) > > > return -EAGAIN; > > > > if (!may_mlock_future(...)) > > > > or > > > > if (!mlock_future_ok(...)) > > > > ? > > > > > > Yeah I struggled with this, because the check only triggers if VM_LOCKED. I was > originally toying with can_mlock_future() but of course, it also returns true if > !VM_LOCKED... > > I think your suggestion of mlock_future_ok() works well, could you change it to > that? Thanks! Sure. I'll make it a separate patch. From: Andrew Morton Subject: mm/mlock: rename mlock_future_check() to mlock_future_ok() Date: Mon May 22 01:52:10 PM PDT 2023 It is felt that the name mlock_future_check() is vague - it doesn't particularly convey the function's operation. mlock_future_ok() is a clearer name for a predicate function. Cc: Liam Howlett Cc: Lorenzo Stoakes Cc: Mike Rapoport (IBM) Cc: Vlastimil Babka Signed-off-by: Andrew Morton --- mm/internal.h | 2 +- mm/mmap.c | 8 ++++---- mm/mremap.c | 2 +- mm/secretmem.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) --- a/mm/internal.h~mm-mlock-rename-mlock_future_check-to-mlock_future_ok +++ a/mm/internal.h @@ -576,7 +576,7 @@ extern long populate_vma_page_range(stru extern long faultin_vma_page_range(struct vm_area_struct *vma, unsigned long start, unsigned long end, bool write, int *locked); -extern bool mlock_future_check(struct mm_struct *mm, unsigned long flags, +extern bool mlock_future_ok(struct mm_struct *mm, unsigned long flags, unsigned long bytes); /* * mlock_vma_folio() and munlock_vma_folio(): --- a/mm/mmap.c~mm-mlock-rename-mlock_future_check-to-mlock_future_ok +++ a/mm/mmap.c @@ -182,7 +182,7 @@ static int check_brk_limits(unsigned lon if (IS_ERR_VALUE(mapped_addr)) return mapped_addr; - return mlock_future_check(current->mm, current->mm->def_flags, len) + return mlock_future_ok(current->mm, current->mm->def_flags, len) ? 0 : -EAGAIN; } static int do_brk_flags(struct vma_iterator *vmi, struct vm_area_struct *brkvma, @@ -1149,7 +1149,7 @@ static inline unsigned long round_hint_t return hint; } -bool mlock_future_check(struct mm_struct *mm, unsigned long flags, +bool mlock_future_ok(struct mm_struct *mm, unsigned long flags, unsigned long bytes) { unsigned long locked_pages, limit_pages; @@ -1275,7 +1275,7 @@ unsigned long do_mmap(struct file *file, if (!can_do_mlock()) return -EPERM; - if (!mlock_future_check(mm, vm_flags, len)) + if (!mlock_future_ok(mm, vm_flags, len)) return -EAGAIN; if (file) { @@ -1927,7 +1927,7 @@ static int acct_stack_growth(struct vm_a return -ENOMEM; /* mlock limit tests */ - if (!mlock_future_check(mm, vma->vm_flags, grow << PAGE_SHIFT)) + if (!mlock_future_ok(mm, vma->vm_flags, grow << PAGE_SHIFT)) return -ENOMEM; /* Check to ensure the stack will not grow into a hugetlb-only region */ --- a/mm/mremap.c~mm-mlock-rename-mlock_future_check-to-mlock_future_ok +++ a/mm/mremap.c @@ -775,7 +775,7 @@ static struct vm_area_struct *vma_to_res if (vma->vm_flags & (VM_DONTEXPAND | VM_PFNMAP)) return ERR_PTR(-EFAULT); - if (!mlock_future_check(mm, vma->vm_flags, new_len - old_len)) + if (!mlock_future_ok(mm, vma->vm_flags, new_len - old_len)) return ERR_PTR(-EAGAIN); if (!may_expand_vm(mm, vma->vm_flags, --- a/mm/secretmem.c~mm-mlock-rename-mlock_future_check-to-mlock_future_ok +++ a/mm/secretmem.c @@ -125,7 +125,7 @@ static int secretmem_mmap(struct file *f if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0) return -EINVAL; - if (!mlock_future_check(vma->vm_mm, vma->vm_flags | VM_LOCKED, len)) + if (!mlock_future_ok(vma->vm_mm, vma->vm_flags | VM_LOCKED, len)) return -EAGAIN; vm_flags_set(vma, VM_LOCKED | VM_DONTDUMP); _