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 7915FC02193 for ; Tue, 4 Feb 2025 18:56:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E51396B0089; Tue, 4 Feb 2025 13:56:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DD9F76B008A; Tue, 4 Feb 2025 13:56:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7C346B008C; Tue, 4 Feb 2025 13:56:27 -0500 (EST) 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 A81166B0089 for ; Tue, 4 Feb 2025 13:56:27 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id C2E701A0A3D for ; Tue, 4 Feb 2025 18:56:26 +0000 (UTC) X-FDA: 83083167972.15.8049863 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf23.hostedemail.com (Postfix) with ESMTP id E7C7D140017 for ; Tue, 4 Feb 2025 18:56:24 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=StCO5zNU; spf=pass (imf23.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738695385; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=zkZpR+3TpNWyT96evNZv0fwxP/Ma/oydjj2Z1L9kqaI=; b=Z1NvelV9STYr5qjrnqs2/bzGKv6AbIQAQSHOafkmf4Jmc0Xl6emvBZ8qBTAXJaOBjbzlYH +fFBXu4azkmia3mSadap/e6dCVlkixyzUFlFpfo1FnbO+afyUnEIoOocB0rnvUCDGP9BlI ba3lpjIPNRmbkBnqN95v4BmDEePGBgY= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=StCO5zNU; spf=pass (imf23.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738695385; a=rsa-sha256; cv=none; b=gd2c9aHiMOSDKDTWyIdUY3G2v+UwCX9lbN61I9XL7zQtOV57tVEMYxkWZrK6CytdgWQ18H oTdQ5jOnuz9V4n4r/zZgGFS62zXzEhJIsnapq7ehgs9a+C5xivsDt8hQuWtxmi5SG6kC+Q cU16ualzhS4upRkLaiazsVkYqN3Mi0o= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id EA89C5C65CB; Tue, 4 Feb 2025 18:55:43 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4389AC4CEDF; Tue, 4 Feb 2025 18:56:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1738695383; bh=caEGT4elt1SHAMsy7JXR43OhY3cX9tXPn5PzzCPAheA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=StCO5zNUL05c6eVpQzojHrRkgc5ouUfv5IpiTssi/rVOK42X9iXn3d9h4miPtNi8P ajJn4OTjLDaPn/TPYKotpWevNRR6XcqqE0uVvAqjXQHzChzDb/LNJbAE+aRyzCfmQx GCNwXD8G0TZmq6QohSv6EK+88hgAEfVyfa0RBDrxuofwgcY4Rfv9Vjx/i4fXUxBFAs tSQtduXTVjSR6umO53axmTnte52GtqoLLFoN1iGolrHMRF7b0mgWMSjM7VFaB/3q2M xM7PUAJSE8Q3Z8jggFA8tDy5PvErAB9jr8IZbmB4EROmofXuC5X2ld1xNjnD8CYW6Y 3Sl3MZ8pu4h2w== From: SeongJae Park To: Lorenzo Stoakes Cc: SeongJae Park , "Liam R. Howlett" , Andrew Morton , David Hildenbrand , Shakeel Butt , Vlastimil Babka , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH v2 4/4] mm/madvise: remove redundant mmap_lock operations from process_madvise() Date: Tue, 4 Feb 2025 10:56:20 -0800 Message-Id: <20250204185620.15928-1-sj@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: E7C7D140017 X-Stat-Signature: x1dezw3tort98npcqsij1aet54gzsbdb X-Rspam-User: X-HE-Tag: 1738695384-634770 X-HE-Meta: U2FsdGVkX1+9zBKjPRp+64hycC4/mrJ2AJ1GCRjyEUIwsoB8R9SBLG7eGB/XtnrtgZxIqYRzWavY7dEBiLMWqsSM56Soo3W1NGHKk0geIZs8nuh5I2JBYy4SwdgieTj0yPGubfAb3ns5VImG+glJ6GtBz9p2aa6wxrYk1fpS6107jtdGENabEiEy4iULYqjpwqXHahHq/OOF9VJYBT0qNJd7jfhmkzoTl9i0+iMRvAp+ZYbJrfR9TBk56VRBQywvHC0TToWYaZPkVdFEDpRsIYGCjEL3kZs728BBR//OyJF96g/wokepMytCJYeukNxTGpQn9/MvbZeyktPAcUPnFEywGWKR7D5DveTr6kQFNYw/OtAQt7ATKG9A0d2RKp6IUk34hy9sDGTcEeLv94fS6lKHyZw3zeOm8Pm2CMISrjtHeyBf1N7rApO1EnOiKPdMMa0V9ec9vaGa6vsk+ez1HOItRnK+QwbQYFbSqud0eGnFuXuArsFzc4xdfrNrGfnPpsCf797EY9FOzJuXph6tVXAPwVODzfstJYAP7ijgHcDIwk/ePEXH8pus4irpE3W+rC72Ne6Pmws65bzmHBh0bDIE7F119gqDb3CMqWwetRE6RsyUbYmzVblQq+1Fd2KCbCaNzuEsMSiWcqSNZxxXK/9GnQIVQl3wrG5qbyxMW1Vr9OxSqSkARdkkoj9DGPKgVl/NnPIDjp/jss0DlsymFzJvkKGzJHilqXCIE5X4wzRj0tMFb3QN6RmNoZlzDlGXnOxBz1nFdRm3fseL4v1iDtA6I/5b4DFMK4W2kJThRFKuav9IKQte7jVor+GPjqJDiDFS2EWcbjBh1/pNgzIyNnyrETHBvBxBq8RE7G19nB05yyvc5eyZ1PdZeANAnFRe2mJoqLCj+nUO24RxxNW0EITaXyT+NsSauoJXuK+A1qhPHckhRGiMTKIrjHNDJ4QF7O5kyvm1xc/8zuEukkH QanuKrG+ bX/FPqF9ecsxtZCLp4sbyi8Y18GX+Eb3sqLn+7IEM0cdXLG97f2k3IgKDGeoNkDW+XJsV7q8nZ+lbv0fz1zRZyvUZk4yj0V7dzLgyRAE7PVIFNAjHM7OfiEzTYlMpQBfQU5pMNRrneaT1YXF8vHUwsXCvV21e8yp/hhoX7a7p5q8P/7LybIkQ25KYQx1mFw8mH0romRECChbMNbhXpRqjoY9KGQLfw2IoXonCfJIyV7Q/5ivSZcn6bnG2lMGL+LCzcddw15EDVkgBAb9o+Fqy8K7zIrDgQbufPTKfGpkS5DUx5yA= 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 Fri, 31 Jan 2025 16:53:17 +0000 Lorenzo Stoakes wrote: > On Thu, Jan 16, 2025 at 05:30:58PM -0800, SeongJae Park wrote: > > Optimize redundant mmap lock operations from process_madvise() by > > directly doing the mmap locking first, and then the remaining works for > > all ranges in the loop. > > > > Signed-off-by: SeongJae Park [...] > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -1752,9 +1752,26 @@ static ssize_t vector_madvise(struct mm_struct *mm, struct iov_iter *iter, [...] > > /* > > * An madvise operation is attempting to restart the syscall, > > * but we cannot proceed as it would not be correct to repeat > > @@ -1776,6 +1793,7 @@ static ssize_t vector_madvise(struct mm_struct *mm, struct iov_iter *iter, > > break; > > iov_iter_advance(iter, iter_iov_len(iter)); > > } > > + madvise_unlock(mm, behavior); > > > > ret = (total_len - iov_iter_count(iter)) ? : ret; > > So I think this is now wrong because of the work I did recently. In this code: > > /* > * An madvise operation is attempting to restart the syscall, > * but we cannot proceed as it would not be correct to repeat > * the operation in aggregate, and would be surprising to the > * user. > * > * As we have already dropped locks, it is safe to just loop and > * try again. We check for fatal signals in case we need exit > * early anyway. > */ > if (ret == -ERESTARTNOINTR) { > if (fatal_signal_pending(current)) { > ret = -EINTR; > break; > } > continue; > } > > Note that it assumes the locks have been dropped before simply trying > again, as the only way this would happen is because of a race, and we may > end up stuck in a loop if we just hold on to the lock. Nice catch! > > So I'd suggest updating this comment and changing the code like this: > > if (ret == -ERESTARTNOINTR) { > if (fatal_signal_pending(current)) { > ret = -EINTR; > break; > } > > + /* Drop and reacquire lock to unwind race. */ > + madvise_unlock(mm, behaviour); > + madvise_lock(mm, behaviour); > continue; > } > > Which brings back the existing behaviour. Thank you for this kind suggestion. I will update next version of this patch in this way. > > By the way I hate that this function swallows error codes. But that's not > your fault, and is now established user-facing behaviour so yeah. Big sigh. > > > > > -- > > 2.39.5 Thanks, SJ