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 162E3C7EE22 for ; Mon, 15 May 2023 15:48:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 298B7900003; Mon, 15 May 2023 11:48:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 24961900002; Mon, 15 May 2023 11:48:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1115C900003; Mon, 15 May 2023 11:48:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 010CC900002 for ; Mon, 15 May 2023 11:48:25 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id B604E1611D6 for ; Mon, 15 May 2023 15:48:25 +0000 (UTC) X-FDA: 80792921370.04.FB7A85A Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf07.hostedemail.com (Postfix) with ESMTP id A610940009 for ; Mon, 15 May 2023 15:48:22 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=K33wzQtv; spf=pass (imf07.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684165702; a=rsa-sha256; cv=none; b=m9vGDLtzfEtAHKEDnl3IDqoLbwLygRXo+AsKh7H1w8EOdedEHq3aCAlvZ4C4LLVClCcjXM tpN7BGhmb1QRpuSZ8GwlYKVLhgb3cqXpNBh60c2RIbPcTiCqviile4XA/sHtPr7+wy7/UK 371kmuB867mN9bUASjaG9GYYDA4kHC0= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=K33wzQtv; spf=pass (imf07.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684165702; 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=C/AmINL4tMPA1IYAOYGHnj9N13Le1qyoYJvFHckB0ME=; b=D78GVeCXcu0NVAbzNj+u9qSVbdcmEdsYtN/xFQQrD7cgtJfBknHSJovNUaX8Dw5lbk/kP3 XzddNsD2EyNmgz8LV9bbPXFc42kGMqLLQZ+c4H6UrVvPYfirvlKyrYseAj7Yz3mRjia91l DlG8Xu37zkSS5E4E4k/6UQU7eXGpntI= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1684165702; 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=C/AmINL4tMPA1IYAOYGHnj9N13Le1qyoYJvFHckB0ME=; b=K33wzQtv4V9SOEk9aPgbtc5CYekEs8Okt7YqCUefU6mdcdiYzknQUuA/6jAiBHlcntGAR2 eoqxtZjqiNWc+T2d6jCxHa3fLN2Ekh570kLnyRb/ZZV1TiexRsK19tpkr/0nYrNJABLfWd oGeUZDRQNBY1jWeeHU8R/ZCacRRPIQg= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-547-X38yFqcePGGip5X-l2BI_w-1; Mon, 15 May 2023 11:48:20 -0400 X-MC-Unique: X38yFqcePGGip5X-l2BI_w-1 Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-757829463dfso30483685a.0 for ; Mon, 15 May 2023 08:48:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684165700; x=1686757700; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=C/AmINL4tMPA1IYAOYGHnj9N13Le1qyoYJvFHckB0ME=; b=hzaHeiXUXpUUqAKwN9wNxT3/TldvFU89gjEgrg3v2nt0yFiPRndQ6ZXH/2wCOqMfJ0 /rUzglaAyhKP4+cjCAnVtMpgbAzEzLMvt6EGz5m9k07e4SNbQKPuDmKTbcjooQ09WeLA jVJLi3ODCBxmDMsDBNCJgY2NagohAN65HefGNOyRXBP7LSMsvfT6qvHY7i093ZYxma8a 61erxV0sQW4Zu2GsO2FQp4YFZHb8OBNz9VKO3gzBVZ9/0CeKhXUiAeRZjfwykso7rVrw sOJ+TbVUYZ55oL5Y9fHXbV3hN17rE7DcydwucJXsP8zmyFcvZAHY8Pdm267udLMDtobP 7bIg== X-Gm-Message-State: AC+VfDzNDkXjSCBptV2MG3AwFBRGgP4cfoQUEIg8q13Mq3BZKDa/maOi tln+6ZTHZBVQCpUwvZ+F5DS7kBpB6tbY6/bxl/wPqotr7kfsTmuUKDpYFVnJhSYwrBLOn/3q7p7 4CVZYK7jD/fI= X-Received: by 2002:a05:622a:508:b0:3f5:1def:67fd with SMTP id l8-20020a05622a050800b003f51def67fdmr10587289qtx.2.1684165700009; Mon, 15 May 2023 08:48:20 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7nE1rgdvC+MTyt8L8N+MkclX/xGEuHVV8DtNlREmY8pvvsSrKBMLd6hHFUfy4s7sUBq78AsQ== X-Received: by 2002:a05:622a:508:b0:3f5:1def:67fd with SMTP id l8-20020a05622a050800b003f51def67fdmr10587251qtx.2.1684165699672; Mon, 15 May 2023 08:48:19 -0700 (PDT) Received: from x1n (bras-base-aurron9127w-grc-62-70-24-86-62.dsl.bell.ca. [70.24.86.62]) by smtp.gmail.com with ESMTPSA id w41-20020a05622a192900b003f4def78743sm4184354qtc.91.2023.05.15.08.48.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 May 2023 08:48:19 -0700 (PDT) Date: Mon, 15 May 2023 11:48:17 -0400 From: Peter Xu To: Jason Gunthorpe Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, John Hubbard , Andrew Morton , "Kirill A . Shutemov" , Lorenzo Stoakes , David Hildenbrand Subject: Re: [PATCH] mm/gup: Fixes FOLL_UNLOCKABLE against FOLL_NOWAIT Message-ID: References: <20230512003102.3149737-1-peterx@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: A610940009 X-Stat-Signature: z567487k6u4moucrjrfx9tie4sxeiqu4 X-HE-Tag: 1684165702-907743 X-HE-Meta: U2FsdGVkX19Ya2oCSRfOyPe0e6NxhFjjWoMVYG8PjDV2TbSCrrPPSZYKijMNKJqYTdGmBhzRp0Ki3Op5a3AVACJgZ/9J3JHEk9QU3t8Tp3K7jbLpOqOqao6danXN4Omd/lo9BzgQaw0GVZFNlBCece+lxA4ZWKFOUP5oIIJaCXalh7kfEnnH4rnP701WmoODUPRQMdVTfv0hPVoeI/ypex7S5/Kz/Z8rEj10JmBpGzKcIaWMJwRi0svB+1IHuxwK8b0NxX3/3xZiljigPzOwLWBA1oJhilsoMOOEd8quC0eVk8NFDALXmIu/VQE9oBVeIneGdRNrtdp+1mMuor0UYkPC9vhJRN3/g8sxen3iuuFacXgkuutLVECF5qMacJ+XXWxUV5D0wtwCGqBY3AaBceExNmgMSOvYc1W9WuebYB1CVa36rjivFY7kGUnBHRJJiaFQeTXAgLSkZGyCuyXqxVpUmf3u6tycE0UGdl+Ylnu1vKDBBYuzSjKkizTxR9btzpdwkr7JwtbwBHoiBc6TdyjHyM9JB3y1Y5LvT6NcBviE9zJpcYdgNnTz3Ez3wc7NojgqaqnkKTaNr2KQ4E4mX1ZxqT4BZdsStrI4Rtbie9NBEt3CKz+eSwJEXNsdBEPD/v1zHmaohk29VT0g4aUZzmXf6CwyJqs+IGXW4cMcAdB4WgIZPuu+tRKmJzPThJRCVXATQ1cXkxeZCbZsffI7ACUvxf4islQvbId28J1XskJsVUrjGkjwy2MXjNPiavfVwiX4z7CAQPgHCjREZZQ1bluEBilmL9ZJ4Y0J5LLiYM5k2vxtmVClRermokhl0/fNAabBA9Zr6VD2LhNl7e1eUtIsBvXk+wkWjdycDk6+q9SigP6SIqXpfzGkvLjAxA8QFMjxffV9OtOrvsUDW6uBrqKs9LpLHLrkEavLM8au44Fa1tHEewMIWMp3XylTs7RBo8DwBx4UM+QFmfSt1VN svSPV+6C OzgwecpildXcOITlliukOtc1nissiYqrxWoEyWMjvG2CisX5ClB/+mPTldvC0TSxCeIrUiyCgFWA+8Wx7sDyFk3WR5TYp3ZxNNQLWfvyWBHE5nUwZa4tvGu9sBeJA+8OEDB5qCCk1c+4VdUFbf8n3G+IDIeMJIHI+eCdZ0mjay2rcQpceqJm0k07AcXWC51ghBKyGdj6s7MDKEz32CMDJdof0q8Rnu6AMamqsh9IzTfgHlXf8MLcphOGUQ7XuSm6cdgoDM+Kr7oQo4b5sePiBmYu1czhb6eQOMv6nggS7Gu+rlbPJO0SkevXK5c8laXVl6CaCALEbI8yr6krqyk+2TGudf6RaMz39fx3rkZZ6FBe6QU9YXF0YLmGrffQ8WVS0uENJCKk30vSZcfrzSvCdsNtgYq3czYp6gIyF X-Bogosity: Ham, tests=bogofilter, spamicity=0.000154, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, May 12, 2023 at 02:06:36PM -0300, Jason Gunthorpe wrote: > On Thu, May 11, 2023 at 08:31:02PM -0400, Peter Xu wrote: > > > E.g., with current code we could at last have FAULT_FLAG_RETRY_NOWAIT set > > even if with a FOLL_UNLOCKABLE gup which doesn't make a lot of > > sense. > > I would say NOWAIT and UNLOCKABLE are different things. UNLOCKABLE > says the mmap sem is allowed to be unlocked, which is true, and NOWAIT > says it shouldn't "wait" (which is something more nebulous than just > sleep). In FOLL_ flag terms it would be fine if the mmap sem was > unlocked while doing NOWAIT - even though the fault hanlder will not > doe this. > > The only caller is fine with this too. > > !UNLOCKABLE literally means not to ever drop the mmap lock which is > not something KVM needs at all. The problem is FOLL_NOWAIT implies FAULT_FLAG_RETRY_NOWAIT internally. Then we'll have FAULT_FLAG_RETRY_NOWAIT+FAULT_FLAG_KILLABLE which makes it very confusing, because RETRY_NOWAIT means we never release mmap lock or retry, then KILL means "if we wait, allow us to be killed". Considering FOLL_UNLOCKABLE is an internal flag while FOLL_NOWAIT a public (even if only with a single caller...), I'd still think it makes more sense and cleaner to just remove FOLL_UNLOCKABLE if FOLL_NOWAIT, no? Again, nothing to blame for previous commit (I explained in the commit message too that we don't need fixes, but simply a cleanup), but it seems removing this confusion of NOWAIT+UNLOCKABLE could be helpful to me. > > So I'd say it is fine as is. A caller should never assume that calling > an unlocked function or passing null locked means that the mmap sem > won't be unlocked while running indirectly because of other GUP > flags. If it wants this behavior it needs to ask for it explicitly > with a locked GUP call and a NULL locked. > > > Since at it, the same commit added unconditional FOLL_UNLOCKABLE in > > faultin_vma_page_range(), which is code-wise correct becuase the helper > > only has one user right now and it always has "locked" set. > > Not quite, it is correct because that is the API contract of this > function. The caller must provide a non-NULL locked and non-NULL > locked at the external interfaces always mean it can be unlocked while > running. Hmm yes, that's the contract. But then it makes more sense to assert on that contract (by checking locked)? How about I rework the commit message but keep the change (which literally only add the assertion)? -- Peter Xu