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 DD75BC636CC for ; Tue, 7 Feb 2023 11:33:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5A2816B00B5; Tue, 7 Feb 2023 06:33:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 552756B00B7; Tue, 7 Feb 2023 06:33:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3F2966B00B8; Tue, 7 Feb 2023 06:33:56 -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 2F3426B00B5 for ; Tue, 7 Feb 2023 06:33:56 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id D9D3380D2A for ; Tue, 7 Feb 2023 11:33:55 +0000 (UTC) X-FDA: 80440286430.19.A2A982C Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by imf13.hostedemail.com (Postfix) with ESMTP id 0A83C20012 for ; Tue, 7 Feb 2023 11:33:52 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=ibm.com header.s=pp1 header.b=gSxODIJ7; spf=pass (imf13.hostedemail.com: domain of imbrenda@linux.ibm.com designates 148.163.156.1 as permitted sender) smtp.mailfrom=imbrenda@linux.ibm.com; dmarc=pass (policy=none) header.from=ibm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1675769633; 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=7Qunp44+S7sMOWgdIyCis4fHKFcFUGjXFxHnNI8CSgs=; b=NG/+uEftq/MwFl2+rS0IaDn+BTtapvOusQRqmMnA/foumENK2vmGIoIPgieFATe8060Vzc efpnFlt65P/YUCWHGYQ5JmzOe/q7lWRlYfQc4/DA7vVPRnFDUK8PbNyAyG4HyfT3btECEu ttESaCy8P5rw0UmRMsW5qWkeGrz9tOY= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=ibm.com header.s=pp1 header.b=gSxODIJ7; spf=pass (imf13.hostedemail.com: domain of imbrenda@linux.ibm.com designates 148.163.156.1 as permitted sender) smtp.mailfrom=imbrenda@linux.ibm.com; dmarc=pass (policy=none) header.from=ibm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1675769633; a=rsa-sha256; cv=none; b=J8Qlvag8CVd2LRLFFrxp3V6fhNHfnuKy9kxC42XYiscXZ0txLgsg5COCb3gVOseznM7Zo8 RaxYEei2RwXufbclJBneVPX5LuYyDOLXoKRf/V+3ieJGtB4qzLbGDULfxY2rcLQUBSaT5G Dj3JwFbL+O4McOskbb/t/ngPXui7RUg= Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3179BxHb010856; Tue, 7 Feb 2023 11:31:21 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=date : from : to : cc : subject : message-id : in-reply-to : references : mime-version : content-type : content-transfer-encoding; s=pp1; bh=7Qunp44+S7sMOWgdIyCis4fHKFcFUGjXFxHnNI8CSgs=; b=gSxODIJ711483Nxm0gwliy0aBJuxdTBzg3ISp2niDm8vDmEy1p7tDL9aFc8r4xg5NaF4 pwJjEKQPm4nCOuBXcPA/kn1/mvUSIjwR6xL0ORUa3VYHFH6dr69BX2wscpRP6ehLLI5u I+r+MYq8ahYuY3yNuT31baLqB8/FyUURX1BWN4cGRp76PbvymAcb0tJSMPdOR44D/xz6 aHpQ4LIiA8W07RcWnpsRcZ8qOzRV2dJ4kr/DMZexyBC06mwDfnSdYsashxywQj6dneSC yitZ8ZtFJc7GgUNiIL9iShQca3oU7Cts90PuQWfwTUpr0/R+iNHzq+LoBrEciJALgjVH OQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3nkksybkdx-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 07 Feb 2023 11:31:21 +0000 Received: from m0098410.ppops.net (m0098410.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 317BAkcG021351; Tue, 7 Feb 2023 11:31:21 GMT Received: from ppma06fra.de.ibm.com (48.49.7a9f.ip4.static.sl-reverse.com [159.122.73.72]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3nkksybkcs-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 07 Feb 2023 11:31:20 +0000 Received: from pps.filterd (ppma06fra.de.ibm.com [127.0.0.1]) by ppma06fra.de.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 316EUuUq017531; Tue, 7 Feb 2023 11:31:18 GMT Received: from smtprelay03.fra02v.mail.ibm.com ([9.218.2.224]) by ppma06fra.de.ibm.com (PPS) with ESMTPS id 3nhemfjh36-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 07 Feb 2023 11:31:18 +0000 Received: from smtpav06.fra02v.mail.ibm.com (smtpav06.fra02v.mail.ibm.com [10.20.54.105]) by smtprelay03.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 317BVEaj47513924 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 7 Feb 2023 11:31:14 GMT Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 617A920043; Tue, 7 Feb 2023 11:31:14 +0000 (GMT) Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0A7E920040; Tue, 7 Feb 2023 11:31:14 +0000 (GMT) Received: from p-imbrenda (unknown [9.152.224.56]) by smtpav06.fra02v.mail.ibm.com (Postfix) with ESMTP; Tue, 7 Feb 2023 11:31:13 +0000 (GMT) Date: Tue, 7 Feb 2023 12:31:12 +0100 From: Claudio Imbrenda To: Jason Gunthorpe Cc: David Hildenbrand , Alistair Popple , David Howells , Christoph Hellwig , John Hubbard , linux-mm@kvack.org, "Mike Rapoport (IBM)" , Christian Borntraeger , Janosch Frank Subject: Re: [PATCH v2 13/13] mm/gup: move private gup FOLL_ flags to internal.h Message-ID: <20230207123112.6900e10b@p-imbrenda> In-Reply-To: References: <13-v2-987e91b59705+36b-gup_tidy_jgg@nvidia.com> <20230126154148.2442e4cd@p-imbrenda> <7388b6a6-85f5-2e61-e3dc-54de531308d0@redhat.com> <20230126163902.16898798@p-imbrenda> <20230130192104.41c5b426@p-imbrenda> Organization: IBM X-Mailer: Claws Mail 4.1.1 (GTK 3.24.35; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: XDsgl0Z957Z1ElLzXXk4OdSPs25riZ3N X-Proofpoint-ORIG-GUID: OoBIoogTzQqjfDrtMfTO1XmbRTBTUAuv X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1 definitions=2023-02-07_03,2023-02-06_03,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 mlxscore=0 spamscore=0 mlxlogscore=630 suspectscore=0 lowpriorityscore=0 bulkscore=0 malwarescore=0 phishscore=0 clxscore=1015 adultscore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2302070103 X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 0A83C20012 X-Stat-Signature: 1ibtx1n8qqtresn4geh1bcahskuhmep5 X-Rspam-User: X-HE-Tag: 1675769632-286851 X-HE-Meta: U2FsdGVkX191CSc0eWG+Msikg+l1C18BiZPdMmeMLoCsQX7ph2GtMT88ilyBqkDRlFvdWWZl5acMdmAq5OPjdBI1+9ZqXHaeIVDzF+BAqiPO6rDvGwdbmUql/DzQIt7wCrV0loKYJGTHN6jAgnKcY1rwsu76Ax4P+WxQP+Jv4+fq27gh0AXCuKsFibfk/U05ks47zUMzWQfgvi5nMGF1YCZDGanSh5tp9UexpglECnRqx6LeII5cLGCRrKio+TeSU2d1QR/I9stthIEUbRcj/mDt7EoQUM8vY79ZD3I6aWTwted6y2ZfwAcCsiv3YyMXqH8GTSvd3Q0BX+AjnvarSQDQNPIPrbcY0ZBj9q8rfBvxmPJWF7aa/gDj6bNWXufVLI/+I4iC8wAyeqvM7zGvXm4hSN5HJhXgKROjXUj0+anTWkC+o0Ni9wzeIOtFXGMBW5KeWlBbstK7okCi1AfyFu9y2s4VfJ88+yPccdtU7AEzCqRvpYkmHcjaWPdXbWEIbZ8sCToioUVOm+b2laT90qOAu8rftDOkZ6Wx05H/6g1Ovzt6Xr9STZK0+P8EmFjuXDuKKqWMo59y53Lwz5hxsp8yIuxym65rnEAvALejSyWRSFy4bGGPppB4IEaqBexlrFRYjqomjU4aUfv6bSFm+dRXjEG/5Ru/+e4vACsj6bQttRe4usCDOn+EK3lEc25a77JRgr72ZX82Y8MJj8DAgAQ7vfefLGmfOTB7tYNX/2I+MsCFLGbrpfK+Y9wQ420P1+GXitmPpD8Mwsa8vmzNip05j0qp+BcluPkftAjJbbmm2fkj6H+pdw/q9WyBx2d7haP7pospS8gF+YT2EcEeXKRwpDZ4/RCi66PMnnKVMzi3TBKsOKbKE5UMGmFPYYO86Gi97d7typOWoDV7E9VjsBkF6Nx4A1Qw0DJhOnvPvlgU9n4NhBmSikYgz6XhxVTE094T+bOZ/yVMZ4CpVqz hxVMoezd A4pjQwsMYaveClXDxllvp5xTWLbGdC+HKnw5hQ4dmajO/50bN5nfWqsu5uhSnRFeA8+y/spIYycYrDEErIvVhszaIMLsqIdpwjxcu6r74GRNtIn3M6IRMT4HT8IhsyJOO3FD8U90G7hedEgcMZwbZBsZ5hIhGk3S8qgBC1uqtyN9SoWl1y1goGbldoKVo2Ld+YntVSd0uMljrQciRkH/Z3XPKWWuECJk8zBzhoS592d2zrjNvT6hyGi7YEudxY2FJFb+z 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, 30 Jan 2023 14:24:40 -0400 Jason Gunthorpe wrote: > On Mon, Jan 30, 2023 at 07:21:04PM +0100, Claudio Imbrenda wrote: > > On Thu, 26 Jan 2023 12:35:37 -0400 > > Jason Gunthorpe wrote: > > > > > On Thu, Jan 26, 2023 at 04:39:02PM +0100, Claudio Imbrenda wrote: > > > > > > > I can tell you that the original goal of that function is to make sure > > > > that there are no extra references. in particular, we want to prevent > > > > I/O of any kind to be ongoing while the page becomes secure. (the I/O > > > > will fail and, depending on which device it was, the whole system might > > > > end up in a rather unhappy state) > > > > > > Sure, but if there is concurrent IO you just try again right? It > > > doesn't wait for refs to drop for instance. > > > > > > So make the lock_page work the same way: > > > > the more I look at this, the less I understand why I wrote the code > > like that. I do have a comment, though > > > > > > > > diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c > > > index 9f18a4af9c1319..847ee50b8672c6 100644 > > > --- a/arch/s390/kernel/uv.c > > > +++ b/arch/s390/kernel/uv.c > > > @@ -193,20 +193,11 @@ static int expected_page_refs(struct page *page) > > > } > > > > > > static int make_secure_pte(pte_t *ptep, unsigned long addr, > > > - struct page *exp_page, struct uv_cb_header *uvcb) > > > + struct page *page, struct uv_cb_header *uvcb) > > > { > > > pte_t entry = READ_ONCE(*ptep); > > > - struct page *page; > > > int expected, cc = 0; > > > > > > - if (!pte_present(entry)) > > > - return -ENXIO; > > > - if (pte_val(entry) & _PAGE_INVALID) > > > - return -ENXIO; > > > - > > > - page = pte_page(entry); > > > - if (page != exp_page) > > > - return -ENXIO; > > > if (PageWriteback(page)) > > > return -EAGAIN; > > > expected = expected_page_refs(page); > > > @@ -304,17 +295,25 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb) > > > goto out; > > > > > > rc = -ENXIO; > > > - page = follow_page(vma, uaddr, FOLL_WRITE); > > > - if (IS_ERR_OR_NULL(page)) > > > - goto out; > > > - > > > - lock_page(page); > > > ptep = get_locked_pte(gmap->mm, uaddr, &ptelock); > > > + > > > + if (!pte_present(entry)) > > > + goto out_unlock_pte; > > > + if (pte_val(entry) & _PAGE_INVALID) > > > + goto out_unlock_pte; > > > > I guess we also need to make sure the page was writable? > > FOLL_WRITE made sure of that > > > > so I guess something like: > > > > if (!pte_write(entry)) > > goto out_unlock_pte; > > Probably, that looks like an existing race that it wasn't re-checked :\ > > Jason how do we want to proceed with this? I can write a patch based on the above and see if anything breaks, but it will take some thinking and testing and reviewing before I'll be comfortable with it.