Received-SPF: pass (google.com: domain of linux-kernel+bounces-26405-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 15 Jan 2024 14:41:02 -0400
From: Jason Gunthorpe <jgg@nvidia.com>
To: peterx@redhat.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	James Houghton <jthoughton@google.com>,
	David Hildenbrand <david@redhat.com>,
	"Kirill A . Shutemov" <kirill@shutemov.name>,
	Yang Shi <shy828301@gmail.com>, linux-riscv@lists.infradead.org,
	Andrew Morton <akpm@linux-foundation.org>,
	"Aneesh Kumar K . V" <aneesh.kumar@kernel.org>,
	Rik van Riel <riel@surriel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Mike Rapoport <rppt@kernel.org>, John Hubbard <jhubbard@nvidia.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Andrew Jones <andrew.jones@linux.dev>,
	linuxppc-dev@lists.ozlabs.org,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Muchun Song <muchun.song@linux.dev>,
	linux-arm-kernel@lists.infradead.org,
	Christoph Hellwig <hch@infradead.org>,
	Lorenzo Stoakes <lstoakes@gmail.com>,
	Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH v2 09/13] mm/gup: Cache *pudp in follow_pud_mask()
Message-ID: <20240115184102.GU734935@nvidia.com>
References: <20240103091423.400294-1-peterx@redhat.com>
 <20240103091423.400294-10-peterx@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240103091423.400294-10-peterx@redhat.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?C6Q4bQcf9PvJCs5KvaZDIXKIi+dVQ0rNEfElxA/30WhejkyzRDyadFx0Nx9i?=
 =?us-ascii?Q?wNIY/PWj+4R24SE0/kjIkuIJpc6gIKNaEzA9w1gH9/8RB/YwlGMgH6vCJopJ?=
 =?us-ascii?Q?9zf8Z1tzjS58N5TqaBdtUzxvsJhAnBASsuUqnfi3QC2pDJECYo4GB77loCyI?=
 =?us-ascii?Q?3xyg9Au7G0A+TRaZ1p7ZQgkhQWGuGu0yQLv5nIxmygKBja3a0EkhP/W/8FcM?=
 =?us-ascii?Q?9hyaCgjQXa4Uj9DO8jT5ky7SnNHibF1xZCdVIsVDns9LK9xHGNRCky0KRgRI?=
 =?us-ascii?Q?O8jbnN7DtZTbJUbIyJ973i7zWTMGSsjMikf2PHjpvX4fS+z8GepaCT7UNevc?=
 =?us-ascii?Q?HKTdCF+dHk8d6+7AsMJ5DKMT9bUmEr2pynUj58t/2yiLz+DgMEBLSQskrfXD?=
 =?us-ascii?Q?VxfQUD7vdhz9hxku3HeS6FxAt3FGjCEfEQMzWd098e443PuSKpogXbSEkoeS?=
 =?us-ascii?Q?e+xRyzd/Wn1Ef7RgDnVr3UcxB3vQ/MLbGJWHHuBKBA2ZJW4xIasWxqQB8323?=
 =?us-ascii?Q?cUxvTdGayBGUivQ8pXpB7x7rbONcH/mUSYTQPhMFLvuVsxe+qpu0dNDA4NOa?=
 =?us-ascii?Q?w9Xm39Pj5ad7eVZBl+cAXuUKu+lGanAYsqjLM5oEeeUiiSQWu1uLOvk8R8Oj?=
 =?us-ascii?Q?9Ji6MBeDh7MrlzN/bFCBd5p86nGoFvaCcSivyN1vyAW9F9O1xnZTodFCEFJo?=
 =?us-ascii?Q?V8+zIk76lYWrPChN8n8/KRe/oBsqJ2LLaWKBCbbbm6Q0+COGq+CRUNXPcEVk?=
 =?us-ascii?Q?LUO6gpmoMtgHeo601+3fITaBgBSALDBug8TSujucnx1SC6Bl8iR6k5TIWCcY?=
 =?us-ascii?Q?cjnE9+an+VOGLxAORDh7JTQ2l4wWL9XhIi2mbrKG9sCbrpSdbhJgzFFzHFd+?=
 =?us-ascii?Q?Rcb0pS9B4JEqM8+1scMXmMAJkuHkPG4fRk0SNVmBAU8pA+DwNQd/yOeWTEv1?=
 =?us-ascii?Q?n9JPrC9YSoikeiQY5C0rWkzY7Lk+1Y/XjmvZqj5YZQW6ME278j76Wl7sqB5q?=
 =?us-ascii?Q?GtwbRxNL34WEuFaD2c/+5lfS/gfaH+sUXaiUWk3Hk/9Y109yFNnyocKxulw7?=
 =?us-ascii?Q?3aT/0R3Ux9F4idTFG/kWS3UNlHe9EM8LZ+dwFnzkojJ6gY7QIGwUNLB669xn?=
 =?us-ascii?Q?DwQFtjLZWcAyzr2yOTFZJF0UnsZ8BuzsIn914utQC0eSanvyfmEqC76FJez3?=
 =?us-ascii?Q?sxcXJRjTGF7dDf36MosGMx6zXxIV7Vt8BfZ/EVhCtB2wJpNrLksnpAcAfgKw?=
 =?us-ascii?Q?VTdh2QHxWfEQWDkusJILbRR+Gqc4y6Y2BUssNGccpu2YIH98mJJvNPmoG+FZ?=
 =?us-ascii?Q?qGf9A0SxG5+dBIBGSABPz/qhu+gwLnNJEKbqhjoMhZ1XmCjPvfsJeWx7xayk?=
 =?us-ascii?Q?rydIP8CqvWWEjfSrV7PUG20hfWJuXJbIWb3ix/kWL226kd7T7fdp1qcAkruT?=
 =?us-ascii?Q?1v0OUScOu+3H9jzxiujGPux8wtycqNDXs3lZcSn6/lFO8PY20KNNslB/G1Vo?=
 =?us-ascii?Q?nTUYSflibbto3v7H+ZCicZyGgjioUwe2LbgN4kVTDjZI2FHKi481FnQfiZCC?=
 =?us-ascii?Q?5TT+Qyi5ZpFEqqfO+yxVIezHjtS1lN+63BbWoLpP?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: abc36491-8ea8-4287-4ecf-08dc15f986b8
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jan 2024 18:41:03.2308
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: i3uKmk1oZbZ8NDMj2a4Z/x072dEUACMzUR1yIEo3ddZUPH1dFQfBO5puws2dAukf
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB7792

On Wed, Jan 03, 2024 at 05:14:19PM +0800, peterx@redhat.com wrote:
> From: Peter Xu <peterx@redhat.com>
> 
> Introduce "pud_t pud" in the function, so the code won't dereference *pudp
> multiple time.  Not only because that looks less straightforward, but also
> because if the dereference really happened, it's not clear whether there
> can be race to see different *pudp values if it's being modified at the
> same time.
> 
> Acked-by: James Houghton <jthoughton@google.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  mm/gup.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)

I think we have several more case like this, and I ceratinly agree
code should not access a READ_ONCE variable more than once :(

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason