Received-SPF: pass (google.com: domain of linux-kernel+bounces-47187-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 31 Jan 2024 16:43:16 -0500
From: "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To: Suren Baghdasaryan <surenb@google.com>
Cc: Lokesh Gidra <lokeshgidra@google.com>, akpm@linux-foundation.org,
        linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, selinux@vger.kernel.org,
        kernel-team@android.com, aarcange@redhat.com, peterx@redhat.com,
        david@redhat.com, axelrasmussen@google.com, bgeffon@google.com,
        willy@infradead.org, jannh@google.com, kaleshsingh@google.com,
        ngeoffray@google.com, timmurray@google.com, rppt@kernel.org
Subject: Re: [PATCH v2 3/3] userfaultfd: use per-vma locks in userfaultfd
 operations
Message-ID: <20240131214316.vteh6vzbb3ubdzqf@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
	Suren Baghdasaryan <surenb@google.com>,
	Lokesh Gidra <lokeshgidra@google.com>, akpm@linux-foundation.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, selinux@vger.kernel.org,
	kernel-team@android.com, aarcange@redhat.com, peterx@redhat.com,
	david@redhat.com, axelrasmussen@google.com, bgeffon@google.com,
	willy@infradead.org, jannh@google.com, kaleshsingh@google.com,
	ngeoffray@google.com, timmurray@google.com, rppt@kernel.org
References: <20240129193512.123145-1-lokeshgidra@google.com>
 <20240129193512.123145-4-lokeshgidra@google.com>
 <20240129203626.uq5tdic4z5qua5qy@revolver>
 <CAJuCfpFS=h8h1Tgn55Hv+cr9bUFFoUvejiFQsHGN5yT7utpDMg@mail.gmail.com>
 <CA+EESO5r+b7QPYM5po--rxQBa9EPi4x1EZ96rEzso288dbpuow@mail.gmail.com>
 <20240130025803.2go3xekza5qubxgz@revolver>
 <CAJuCfpF0J_7vgTZim3vfH6=ExRTsCRtpg+beJ+bJfYEqD5Se8g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <CAJuCfpF0J_7vgTZim3vfH6=ExRTsCRtpg+beJ+bJfYEqD5Se8g@mail.gmail.com>
User-Agent: NeoMutt/20220429
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?YkpjNFROLzlLSHpZMk51b2pVaC9NeHBYREJDZWZJa2M4VnRkRWpkeWNVenha?=
 =?utf-8?B?L00xckVYVkpQZ1YyQ0szQUZVeWtsKzJJREtHMFlTaWdHQUorcXd2SENEemRZ?=
 =?utf-8?B?Q1lPZGxDc1RDVFp2WDBSSVRBVzV3Z1V2ZC8vbkZaOXoxcVQ5SmxRY1dBVkNC?=
 =?utf-8?B?N1FtOFdTZUswZmJOQjdYNWZ4YWNHbm5YdWFUQ0RzU2s3OUNTdTR1VDgwZFdt?=
 =?utf-8?B?NUs2dzNKTmQ1VE5WZmhtcUFZR0drbENxRVpoY1QvbG9QRUsxa3VaWFM0WGdp?=
 =?utf-8?B?dkFXODdBTzF5eFNqSnFrbDRiMzFyVlpCOUxVWm5tOS9SY1VuM3ZmTmhQSG9y?=
 =?utf-8?B?Z0EzR0xXTVNiVzBJQzZlSGI1ZHpXQ0lzbjkwcUZkb1VHWjN1NDFFb1cxZFlq?=
 =?utf-8?B?b2UwVDZNM0ptN3B5TnYvR1FHTW1rODRNdUx5S1JGTUYzd0RsL0prb3ZqZXRZ?=
 =?utf-8?B?NWNTbVBYN3JMMDZHVVdGN2RtMnc5N25UQ0xFU0o1ekJhSmhZWEFMQm94djVn?=
 =?utf-8?B?cDNCOXBOa2hTYkFTNFVwQTQzTDdLdy9pWlhYZ2NySFNSUnlEdld6M1gzVjlp?=
 =?utf-8?B?b0VtTmQxbWc4Y0JUMnNhdmgyK3ppTThmRUhNeDd5SEVlOFZIOXNza1d4bWl6?=
 =?utf-8?B?U1Qwdk1rOGkyVllOc0w0dUVJNS9JNjJYNlhYTkFoOXFMcGRiblo1Uk1rVVNz?=
 =?utf-8?B?VHVNL201RlNaWmVXb2llUVlQZFJaY0xXcUN6MVAzSHhpRHhNOWV2YVd3aUVa?=
 =?utf-8?B?UEtYbEVVMmJVSEJnZFZDQkFldFpIa2pvU04wb2NWVml1T3h0OVhSa3Y0VTNP?=
 =?utf-8?B?aUVaTHUrZmdPK2U0ZDBnOFBjdHpTOUxMRktYSGV2QWZiM2xqSEpkUklyd1VW?=
 =?utf-8?B?NlZFb0VCd25TQ3hHTUhRWWRQUzY3L00xZnpHZW1pK3dGYVRrb3loMnQ1MW83?=
 =?utf-8?B?MjVNYXZlbTBlaG1JQjhlTjd5b0ZtMjNUV0h6QS80U3dnU3FTSWpFeFh6VUFJ?=
 =?utf-8?B?VnRTVVZEWmY2aGtITjNXUFNibkh5OXU2dklVcy92Rk9VTCtKQ0RYVEdscElh?=
 =?utf-8?B?Sis3VjU0MFY3WHZPTk55RkN1NWc2Y1RTcktDbUhoUHZXTUNGT1c4eDhmZmpv?=
 =?utf-8?B?SU01aTVqTEpzM2llNWZHZG00b1MrbXVKaGV6L3cyZG0zVkIvK2hiOHRoTGkz?=
 =?utf-8?B?Q00xUmF2WXB3Z0x0K0FVMVpaampRQzlabktLc1ZqMmhuSlJjS2g0U0VPb0w5?=
 =?utf-8?B?b0RWSVh4azNScUQzL3hwWmh4TWVJamptWGM1VG05NXhGME9PSEpEVUIvZUhR?=
 =?utf-8?B?NkVnUjF6WXBGQ0FOQ1ZRN2dONjVtMjJtTVBIbndjUGtKbDV4M0pMQWRLSkdh?=
 =?utf-8?B?L0FFc2NQSXNTMmtkbnB4Nng3VTNtUXdEbkdXbEtxemhBTGRhM0Jyd1BtRHV2?=
 =?utf-8?B?dkFpdjhqdXhYTmNkMjk4QTRGUjBjRmQzSjJOT2g0dXNxZ1BjQlI2YlB6Rmdz?=
 =?utf-8?B?ekdsY0wwbTRtdnN3YWxRaTExYkg0U0puMThTWG5zQjlaN2owRU52emdFZ1l5?=
 =?utf-8?B?bUpYQ1BOaEcwbm5Tc3JZc1AybDhUcXhWV3NpdWc2bnpyTm1rUGI5d0hTeTNx?=
 =?utf-8?B?UFkxc3ZpWTRXZW5TM1l0aUU2UlVSWlhQMGNMbmw3R29sWjlVNDRDTVUzczB2?=
 =?utf-8?B?Qm5wbnMwWVFScVdLc01weUlSRXYyNU9ISU5haGV2YU9lencvaDA4S1UyaXdB?=
 =?utf-8?B?L0pobHJUQzUySUJmUFc1TjVKVDFMSjhTc2N4QXNCSkxyNHEvV1hpbitLL0lt?=
 =?utf-8?B?YkdzZGJQNFFvQk1FR21nMEp6ZHNVUlVVNWxnQnhleE1hVFlzSFl4K3JrcHhk?=
 =?utf-8?B?MHBwdDIyc2dtazZZOFdmQ1ZadDZsL0dxdlFIZlhKdlpVcEh5WDAvOFVoak9p?=
 =?utf-8?B?QkVrczFRcTJJcXFjbWI1RllTODlQbWFSZW52UEhuTGU3ZU5wZGdOWVd0SHJ0?=
 =?utf-8?B?UndPcmRnYUhQMVRRaEdZQVE1bWhzOVl2MHJsU09GbWVCOS9uOTNQQ09maTc0?=
 =?utf-8?B?Zm04YmVnYVV1WjVTMVlzNVNSQ2dwRzB0bFYzY3JnVlU3R2ZVd2d4ODFrYTJr?=
 =?utf-8?Q?hChM+BRF+cqcOYAewrwSxCulD?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	vqk+JgUIyYqQDja9dJCrL5iLZVeNBAiYsgETpU44zuTvU4yTwYmzYSrwE29MbDCmnUftL6pRjKP4BSVLDJQ+6vhOm+p3U75El2FPEF7BZOyi0RqVAjlTy+aeRmivzi397zKnSDV6hleckk7z9IyJy0E+4grPXltFVUfIxeCDHlGJuT1rvBwi6PURtCEttx+tftWiYPJyAIIRwXtQ5RkJkIvIMVRwIovLVM1lFWFcZimrTnSvk+VpvUKqHRzVbUnadrB3c7ayZ1sd5uwaPsU1ZFCz+iYDkDpIF3t3g6GbMHSz5kK1Vo1S9dTnnGCDXRhDnktdWffbpX7YZBZ52OySRLyFHoEzyTWnFTSby8cJ0Z4EIk3zm98FyU4u6/3/I1frwDqlMVykROGyNqNmzgkXk0VWNGlqmBFm9rKbbDP3ESnziN7lqXk7EE5ldKXmfbd8VWrleSMeWSvtYFO6Zj4+SRulBWHH8DeYVElF01SR4NEPA9KkMG8T83CBT9sXbaDT47XRswPeV37om/E/Lp495OlSnmEE7Lnmon+gV755pHqND8O8VJQsbnPBC7cThNIqF7cN4+iNK0BbsRtNbhlF8lNiVSAX5FnYjSLyFqv4O98=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d11f6c60-f05c-4f27-b1c0-08dc22a5a37d
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7933.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2024 21:43:18.9108
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qlysWkEeZ+J6YLvuqpLST3uvNNDq+8B9vWH5h4oSW3hs1PDYvcIWI8ls2uKBDVGWZZ8rFaP8vQYMkqX7TKzFIw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB5671
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-01-31_10,2024-01-31_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 malwarescore=0
 mlxscore=0 mlxlogscore=999 spamscore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2401310169
X-Proofpoint-ORIG-GUID: TxjNT9A8cChat4JaGh6re1rbjpdGoaay
X-Proofpoint-GUID: TxjNT9A8cChat4JaGh6re1rbjpdGoaay

* Suren Baghdasaryan <surenb@google.com> [240130 22:03]:
> On Mon, Jan 29, 2024 at 6:58=E2=80=AFPM Liam R. Howlett <Liam.Howlett@ora=
cle.com> wrote:

..

> > > > > > @@ -730,7 +759,7 @@ static __always_inline ssize_t mfill_atomic=
(struct userfaultfd_ctx *ctx,
> > > > > >
> > > > > >  out_unlock:
> > > > > >       up_read(&ctx->map_changing_lock);
> > > > > > -     mmap_read_unlock(dst_mm);
> > > > > > +     unpin_vma(dst_mm, dst_vma, &mmap_locked);
> > > > > >  out:
> > > > > >       if (folio)
> > > > > >               folio_put(folio);
> > > > > > @@ -1285,8 +1314,6 @@ static int validate_move_areas(struct use=
rfaultfd_ctx *ctx,
> > > > > >   * @len: length of the virtual memory range
> > > > > >   * @mode: flags from uffdio_move.mode
> > > > > >   *
> > > > > > - * Must be called with mmap_lock held for read.
> > > > > > - *
> > > > > >   * move_pages() remaps arbitrary anonymous pages atomically in=
 zero
> > > > > >   * copy. It only works on non shared anonymous pages because t=
hose can
> > > > > >   * be relocated without generating non linear anon_vmas in the=
 rmap
> > > > > > @@ -1353,15 +1380,16 @@ static int validate_move_areas(struct u=
serfaultfd_ctx *ctx,
> > > > > >   * could be obtained. This is the only additional complexity a=
dded to
> > > > > >   * the rmap code to provide this anonymous page remapping func=
tionality.
> > > > > >   */
> > > > > > -ssize_t move_pages(struct userfaultfd_ctx *ctx, struct mm_stru=
ct *mm,
> > > > > > -                unsigned long dst_start, unsigned long src_sta=
rt,
> > > > > > -                unsigned long len, __u64 mode)
> > > > > > +ssize_t move_pages(struct userfaultfd_ctx *ctx, unsigned long =
dst_start,
> > > > > > +                unsigned long src_start, unsigned long len, __=
u64 mode)
> > > > > >  {
> > > > > > +     struct mm_struct *mm =3D ctx->mm;
> > > > > >       struct vm_area_struct *src_vma, *dst_vma;
> > > > > >       unsigned long src_addr, dst_addr;
> > > > > >       pmd_t *src_pmd, *dst_pmd;
> > > > > >       long err =3D -EINVAL;
> > > > > >       ssize_t moved =3D 0;
> > > > > > +     bool mmap_locked =3D false;
> > > > > >
> > > > > >       /* Sanitize the command parameters. */
> > > > > >       if (WARN_ON_ONCE(src_start & ~PAGE_MASK) ||
> > > > > > @@ -1374,28 +1402,52 @@ ssize_t move_pages(struct userfaultfd_c=
tx *ctx, struct mm_struct *mm,
> > > > > >           WARN_ON_ONCE(dst_start + len <=3D dst_start))
> > > > > >               goto out;
> > > > >
> > > > > Ah, is this safe for rmap?  I think you need to leave this read l=
ock.
> > > > >
> > > I didn't fully understand you here.
> >
> > Sorry, I'm confused on how your locking scheme avoids rmap from trying
> > to use the VMA with the atomic increment part.
>=20
> I'm also a bit confused. Which atomic increment are you referring to?
> AFAIU move_pages() will lock both src_vma and dst_vma, so even if rmap
> finds them it can't modify them, no?

The uffd atomic, mmap_changing.

..