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 178F0C19F32 for ; Wed, 5 Mar 2025 21:40:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C87F6280005; Wed, 5 Mar 2025 16:40:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C3435280003; Wed, 5 Mar 2025 16:40:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD5BB280005; Wed, 5 Mar 2025 16:40:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 8EAAA280003 for ; Wed, 5 Mar 2025 16:40:24 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id CA07755C16 for ; Wed, 5 Mar 2025 21:40:26 +0000 (UTC) X-FDA: 83188816452.30.1D040A4 Received: from out-183.mta1.migadu.com (out-183.mta1.migadu.com [95.215.58.183]) by imf08.hostedemail.com (Postfix) with ESMTP id CB5B9160002 for ; Wed, 5 Mar 2025 21:40:24 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=JbGisM6m; spf=pass (imf08.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.183 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741210825; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=lT+rjmxz451RtxK+9XYt2MkibeQON+Ip2eMj4hfzm0Y=; b=lPNTBnOxjDZe7ieE1K1cmdCKrbXPPS13AJAYomc6wJNZs1iM9YcQhgdxXjtrmDEUcrZDO7 IEQOODNHC/f8WdlbA0NX+CBMKma3qokeVv/w49wZei9eJcgmBvUQM9lan403URQHkej36H tNDsAMHkhQAyXbkTXdC/k137wy6QhoQ= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=JbGisM6m; spf=pass (imf08.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.183 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741210825; a=rsa-sha256; cv=none; b=T+VLBG43CX2Kk0jMrHGBln8xL/ByO3FtFrxxrGzrboTKiDFzhWq9Yv91osSNmO+bt+RmMN 6N6D4bXY0mtkFBKnQ/UWgVUgO+68oKuif5jLOaWq5YOMP3RgXAoBrQw0qJgDZaBTcBqaeg 3LTA4y5A7dgVyPVajZjJ2zMTqbCNskE= Date: Wed, 5 Mar 2025 13:40:17 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1741210822; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=lT+rjmxz451RtxK+9XYt2MkibeQON+Ip2eMj4hfzm0Y=; b=JbGisM6mw8n8II39u4BslBwwYGM8UWCoZUjk8OpayK6lW46w7FwnKHu+p/5i7ZsWpan127 jcD0TWuUbv4wfq2B3qsc1v1Ls0qfGokmhnex1yhnnbK+2Fljg93aPjiUhC71y1suzE4Fa6 UV5kZHKRqwZZEB8UOMNeW1g5/I7x0Bw= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: SeongJae Park Cc: "Liam R. Howlett" , Andrew Morton , David Hildenbrand , Lorenzo Stoakes , Vlastimil Babka , kernel-team@meta.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH 05/16] mm/madvise: define and use madvise_behavior struct for madvise_do_behavior() Message-ID: References: <20250305181611.54484-1-sj@kernel.org> <20250305181611.54484-6-sj@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Stat-Signature: pjmfdhdny4omjydq77rsptw5s8dbbir8 X-Rspamd-Queue-Id: CB5B9160002 X-Rspamd-Server: rspam06 X-Rspam-User: X-HE-Tag: 1741210824-864272 X-HE-Meta: U2FsdGVkX18vK9yA02CjcQ09Yp4cgVuhSoHs5v5D7JvzghNdKIyTLhw1+nCHQuCGdCpiPejySxRVNgSRJuo9Ku5De7C64033GMT88fiVhae0dB1fd6a7H03WLCrbq6dGPQhstaOjdGu4obMSoWCvw7PKB8sQjkcl4DaaW44vM8TufZGA2JLCUrWBwVqYeydOO7hdMqR95A4N0wfo5k2nMZKSrqMN/Iq4cburmHOrYo5Xr+wHIdjayqOSio6g6u9xZFdX74tekmqcCRonTINYAD+6KUvV41rtUlgl+OBfJ504Apzb9W1eQ2w+VNKmBbxGwgob63bbN1zLImK+ANW0yw/G7OaqIpw0QpdRn+OW9pT3r1QnNzJRJDCONI7jzTqXdMvUJU6B1i9+fnIY4PJSlNR7YnaNRlp+UA3XyL1w9blLe/0HkZBg2P50a7xkFaZPUB3rvrITc1ZwUHLF/VyiY8/sivfmXpb96/Fo48rFJ7gCL60/x6DS2zQ4WC9ER8cN/KkhLzS6N4skVSF8XhM+EWe3DAdOe1kvrx/V+LKa3PGKtahKLtbo2hj9Jh+mWW0kiuDj+LEGqxjPSLArHrvIgVQZ5bdS/Q1wteO/LELpYJtK+90+/uVM3yBlSY33jiEQ4viGscWAwOgX8cplkfMpzGeYFEvWkzkz0gOAbsEXhPZhfY0ehNdbNGCNQJeZd2dB9W0bbSACFdvZkT6Hi7SdYXf8yVzyPjYlzj+6wVRguryzftmDTkzk4uBL1iN69MKxjUIwqoROk5sARIuumCF9g5Fzfm308p/D2XLwqSPGJHYW43nvzW0A8j9MXuIJ5vtat7SHu2gPfhL0QlT6YD04ML+/0hS+WQQzy2s4dT4VGjXCdrZpMTDWbLlwzHd5hWAIiTkpISyqbpbAXwqrpjTysgxlEoE/XOrZ7/LrHDq0md+pPBxFGxlOVqWWLW4xd4dXAu9Bs4V/4ecHq740EMJ 7D0TkAo6 wlaRt6ao6QRhGYHWQV6jAjodBGNDTHc8uXj1tegM4B/VWMjRq7gg6ZMw8zAbb5n0HVHKGgOKN9pYgQmIlkI3UThi3RVVpa/6wgcoy7EMIdqanakweov4Ox7fgGoZXh5QrDxG74BDeJiKut1H7btpD/Y9uLWcciu4JlAuePDoK5oNaRgtF/cOVmy38Dkcp6B0x5Ltyj99kimU5tOby+72i5l5pgpzVouF+/TGGPBpGyFEyKxOM7OUS319yKw== 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: List-Subscribe: List-Unsubscribe: On Wed, Mar 05, 2025 at 01:02:15PM -0800, Shakeel Butt wrote: > On Wed, Mar 05, 2025 at 10:16:00AM -0800, SeongJae Park wrote: > > To implement batched tlb flushes for MADV_DONTNEED[_LOCKED] and > > MADV_FREE, an mmu_gather object in addition to the behavior integer need > > to be passed to the internal logics. Define a struct for passing such > > information together with the behavior value without increasing number > > of parameters of all code paths towards the internal logic. > > > > Signed-off-by: SeongJae Park > > --- > > mm/madvise.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/mm/madvise.c b/mm/madvise.c > > index c5e1a4d1df72..3346e593e07d 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -1665,9 +1665,15 @@ static bool is_memory_populate(int behavior) > > } > > } > > > > +struct madvise_behavior { > > + int behavior; > > +}; > > I think the patch 5 to 8 are just causing churn and will be much better > to be a single patch. Also why not make the above struct a general > madvise visit param struct which can be used by both > madvise_vma_anon_name() and madvise_vma_behavior()? Something like following: diff --git a/mm/madvise.c b/mm/madvise.c index c5e1a4d1df72..cbc3817366a6 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -890,11 +890,17 @@ static bool madvise_dontneed_free_valid_vma(struct vm_area_struct *vma, return true; } +struct madvise_walk_param { + int behavior; + struct anon_vma_name *anon_name; +}; + static long madvise_dontneed_free(struct vm_area_struct *vma, struct vm_area_struct **prev, unsigned long start, unsigned long end, - int behavior) + struct madvise_walk_param *arg) { + int behavior = arg->behavior; struct mm_struct *mm = vma->vm_mm; *prev = vma; @@ -1249,8 +1255,9 @@ static long madvise_guard_remove(struct vm_area_struct *vma, static int madvise_vma_behavior(struct vm_area_struct *vma, struct vm_area_struct **prev, unsigned long start, unsigned long end, - unsigned long behavior) + struct madvise_walk_param *arg) { + int behavior = arg->behavior; int error; struct anon_vma_name *anon_name; unsigned long new_flags = vma->vm_flags; @@ -1270,7 +1277,7 @@ static int madvise_vma_behavior(struct vm_area_struct *vma, case MADV_FREE: case MADV_DONTNEED: case MADV_DONTNEED_LOCKED: - return madvise_dontneed_free(vma, prev, start, end, behavior); + return madvise_dontneed_free(vma, prev, start, end, arg); case MADV_NORMAL: new_flags = new_flags & ~VM_RAND_READ & ~VM_SEQ_READ; break; @@ -1462,10 +1469,10 @@ static bool process_madvise_remote_valid(int behavior) */ static int madvise_walk_vmas(struct mm_struct *mm, unsigned long start, - unsigned long end, unsigned long arg, + unsigned long end, struct madvise_walk_param *arg, int (*visit)(struct vm_area_struct *vma, struct vm_area_struct **prev, unsigned long start, - unsigned long end, unsigned long arg)) + unsigned long end, struct madvise_walk_param *arg)) { struct vm_area_struct *vma; struct vm_area_struct *prev; @@ -1523,7 +1530,7 @@ int madvise_walk_vmas(struct mm_struct *mm, unsigned long start, static int madvise_vma_anon_name(struct vm_area_struct *vma, struct vm_area_struct **prev, unsigned long start, unsigned long end, - unsigned long anon_name) + struct madvise_walk_param *arg) { int error; @@ -1532,7 +1539,7 @@ static int madvise_vma_anon_name(struct vm_area_struct *vma, return -EBADF; error = madvise_update_vma(vma, prev, start, end, vma->vm_flags, - (struct anon_vma_name *)anon_name); + arg->anon_name); /* * madvise() returns EAGAIN if kernel resources, such as @@ -1548,6 +1555,7 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start, { unsigned long end; unsigned long len; + struct madvise_walk_param param = { .anon_name = anon_name }; if (start & ~PAGE_MASK) return -EINVAL; @@ -1564,7 +1572,7 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start, if (end == start) return 0; - return madvise_walk_vmas(mm, start, end, (unsigned long)anon_name, + return madvise_walk_vmas(mm, start, end, ¶m, madvise_vma_anon_name); } #endif /* CONFIG_ANON_VMA_NAME */ @@ -1666,8 +1674,10 @@ static bool is_memory_populate(int behavior) } static int madvise_do_behavior(struct mm_struct *mm, - unsigned long start, size_t len_in, int behavior) + unsigned long start, size_t len_in, + struct madvise_walk_param *arg) { + int behavior = arg->behavior; struct blk_plug plug; unsigned long end; int error; @@ -1681,7 +1691,7 @@ static int madvise_do_behavior(struct mm_struct *mm, if (is_memory_populate(behavior)) error = madvise_populate(mm, start, end, behavior); else - error = madvise_walk_vmas(mm, start, end, behavior, + error = madvise_walk_vmas(mm, start, end, arg, madvise_vma_behavior); blk_finish_plug(&plug); return error; @@ -1762,13 +1772,14 @@ static int madvise_do_behavior(struct mm_struct *mm, int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int behavior) { int error; + struct madvise_walk_param arg = {.behavior = behavior}; if (madvise_should_skip(start, len_in, behavior, &error)) return error; error = madvise_lock(mm, behavior); if (error) return error; - error = madvise_do_behavior(mm, start, len_in, behavior); + error = madvise_do_behavior(mm, start, len_in, &arg); madvise_unlock(mm, behavior); return error; @@ -1785,6 +1796,7 @@ static ssize_t vector_madvise(struct mm_struct *mm, struct iov_iter *iter, { ssize_t ret = 0; size_t total_len; + struct madvise_walk_param arg = {.behavior = behavior}; total_len = iov_iter_count(iter); @@ -1800,7 +1812,7 @@ static ssize_t vector_madvise(struct mm_struct *mm, struct iov_iter *iter, if (madvise_should_skip(start, len_in, behavior, &error)) ret = error; else - ret = madvise_do_behavior(mm, start, len_in, behavior); + ret = madvise_do_behavior(mm, start, len_in, &arg); /* * An madvise operation is attempting to restart the syscall, * but we cannot proceed as it would not be correct to repeat -- 2.43.5