From: Suren Baghdasaryan <surenb@google.com>
To: kernel test robot <lkp@intel.com>
Cc: akpm@linux-foundation.org, oe-kbuild-all@lists.linux.dev,
viro@zeniv.linux.org.uk, brauner@kernel.org, shuah@kernel.org,
aarcange@redhat.com, lokeshgidra@google.com, peterx@redhat.com,
david@redhat.com, hughd@google.com, mhocko@suse.com,
axelrasmussen@google.com, rppt@kernel.org, willy@infradead.org,
Liam.Howlett@oracle.com, jannh@google.com,
zhangpeng362@huawei.com, bgeffon@google.com,
kaleshsingh@google.com, ngeoffray@google.com, jdduke@google.com,
linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
kernel-team@android.com
Subject: Re: [PATCH v3 2/3] userfaultfd: UFFDIO_MOVE uABI
Date: Thu, 19 Oct 2023 14:55:31 -0700 [thread overview]
Message-ID: <CAJuCfpGJqzuJqfH42=f5LXxkf4uCNmUUh2zXyzodczP29O6_hQ@mail.gmail.com> (raw)
In-Reply-To: <202310180338.zTpcYECK-lkp@intel.com>
On Tue, Oct 17, 2023 at 12:40 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Suren,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on akpm-mm/mm-everything]
> [also build test WARNING on next-20231017]
> [cannot apply to linus/master v6.6-rc6]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Suren-Baghdasaryan/mm-rmap-support-move-to-different-root-anon_vma-in-folio_move_anon_rmap/20231009-144552
> base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> patch link: https://lore.kernel.org/r/20231009064230.2952396-3-surenb%40google.com
> patch subject: [PATCH v3 2/3] userfaultfd: UFFDIO_MOVE uABI
> config: i386-randconfig-141-20231017 (https://download.01.org/0day-ci/archive/20231018/202310180338.zTpcYECK-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20231018/202310180338.zTpcYECK-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202310180338.zTpcYECK-lkp@intel.com/
>
> smatch warnings:
> mm/userfaultfd.c:1380 remap_pages() warn: unsigned 'src_start + len - src_addr' is never less than zero.
Hmm. I think this warning is correct only when
CONFIG_TRANSPARENT_HUGEPAGE=n. I guess I'll have to add an "ifdef
CONFIG_TRANSPARENT_HUGEPAGE" here after all, which lets us move these
checks before locking PTL.
>
> vim +1380 mm/userfaultfd.c
>
> 1195
> 1196 /**
> 1197 * remap_pages - remap arbitrary anonymous pages of an existing vma
> 1198 * @dst_start: start of the destination virtual memory range
> 1199 * @src_start: start of the source virtual memory range
> 1200 * @len: length of the virtual memory range
> 1201 *
> 1202 * remap_pages() remaps arbitrary anonymous pages atomically in zero
> 1203 * copy. It only works on non shared anonymous pages because those can
> 1204 * be relocated without generating non linear anon_vmas in the rmap
> 1205 * code.
> 1206 *
> 1207 * It provides a zero copy mechanism to handle userspace page faults.
> 1208 * The source vma pages should have mapcount == 1, which can be
> 1209 * enforced by using madvise(MADV_DONTFORK) on src vma.
> 1210 *
> 1211 * The thread receiving the page during the userland page fault
> 1212 * will receive the faulting page in the source vma through the network,
> 1213 * storage or any other I/O device (MADV_DONTFORK in the source vma
> 1214 * avoids remap_pages() to fail with -EBUSY if the process forks before
> 1215 * remap_pages() is called), then it will call remap_pages() to map the
> 1216 * page in the faulting address in the destination vma.
> 1217 *
> 1218 * This userfaultfd command works purely via pagetables, so it's the
> 1219 * most efficient way to move physical non shared anonymous pages
> 1220 * across different virtual addresses. Unlike mremap()/mmap()/munmap()
> 1221 * it does not create any new vmas. The mapping in the destination
> 1222 * address is atomic.
> 1223 *
> 1224 * It only works if the vma protection bits are identical from the
> 1225 * source and destination vma.
> 1226 *
> 1227 * It can remap non shared anonymous pages within the same vma too.
> 1228 *
> 1229 * If the source virtual memory range has any unmapped holes, or if
> 1230 * the destination virtual memory range is not a whole unmapped hole,
> 1231 * remap_pages() will fail respectively with -ENOENT or -EEXIST. This
> 1232 * provides a very strict behavior to avoid any chance of memory
> 1233 * corruption going unnoticed if there are userland race conditions.
> 1234 * Only one thread should resolve the userland page fault at any given
> 1235 * time for any given faulting address. This means that if two threads
> 1236 * try to both call remap_pages() on the same destination address at the
> 1237 * same time, the second thread will get an explicit error from this
> 1238 * command.
> 1239 *
> 1240 * The command retval will return "len" is successful. The command
> 1241 * however can be interrupted by fatal signals or errors. If
> 1242 * interrupted it will return the number of bytes successfully
> 1243 * remapped before the interruption if any, or the negative error if
> 1244 * none. It will never return zero. Either it will return an error or
> 1245 * an amount of bytes successfully moved. If the retval reports a
> 1246 * "short" remap, the remap_pages() command should be repeated by
> 1247 * userland with src+retval, dst+reval, len-retval if it wants to know
> 1248 * about the error that interrupted it.
> 1249 *
> 1250 * The UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES flag can be specified to
> 1251 * prevent -ENOENT errors to materialize if there are holes in the
> 1252 * source virtual range that is being remapped. The holes will be
> 1253 * accounted as successfully remapped in the retval of the
> 1254 * command. This is mostly useful to remap hugepage naturally aligned
> 1255 * virtual regions without knowing if there are transparent hugepage
> 1256 * in the regions or not, but preventing the risk of having to split
> 1257 * the hugepmd during the remap.
> 1258 *
> 1259 * If there's any rmap walk that is taking the anon_vma locks without
> 1260 * first obtaining the folio lock (the only current instance is
> 1261 * folio_referenced), they will have to verify if the folio->mapping
> 1262 * has changed after taking the anon_vma lock. If it changed they
> 1263 * should release the lock and retry obtaining a new anon_vma, because
> 1264 * it means the anon_vma was changed by remap_pages() before the lock
> 1265 * could be obtained. This is the only additional complexity added to
> 1266 * the rmap code to provide this anonymous page remapping functionality.
> 1267 */
> 1268 ssize_t remap_pages(struct mm_struct *dst_mm, struct mm_struct *src_mm,
> 1269 unsigned long dst_start, unsigned long src_start,
> 1270 unsigned long len, __u64 mode)
> 1271 {
> 1272 struct vm_area_struct *src_vma, *dst_vma;
> 1273 unsigned long src_addr, dst_addr;
> 1274 pmd_t *src_pmd, *dst_pmd;
> 1275 long err = -EINVAL;
> 1276 ssize_t moved = 0;
> 1277
> 1278 /*
> 1279 * Sanitize the command parameters:
> 1280 */
> 1281 BUG_ON(src_start & ~PAGE_MASK);
> 1282 BUG_ON(dst_start & ~PAGE_MASK);
> 1283 BUG_ON(len & ~PAGE_MASK);
> 1284
> 1285 /* Does the address range wrap, or is the span zero-sized? */
> 1286 BUG_ON(src_start + len <= src_start);
> 1287 BUG_ON(dst_start + len <= dst_start);
> 1288
> 1289 /*
> 1290 * Because these are read sempahores there's no risk of lock
> 1291 * inversion.
> 1292 */
> 1293 mmap_read_lock(dst_mm);
> 1294 if (dst_mm != src_mm)
> 1295 mmap_read_lock(src_mm);
> 1296
> 1297 /*
> 1298 * Make sure the vma is not shared, that the src and dst remap
> 1299 * ranges are both valid and fully within a single existing
> 1300 * vma.
> 1301 */
> 1302 src_vma = find_vma(src_mm, src_start);
> 1303 if (!src_vma || (src_vma->vm_flags & VM_SHARED))
> 1304 goto out;
> 1305 if (src_start < src_vma->vm_start ||
> 1306 src_start + len > src_vma->vm_end)
> 1307 goto out;
> 1308
> 1309 dst_vma = find_vma(dst_mm, dst_start);
> 1310 if (!dst_vma || (dst_vma->vm_flags & VM_SHARED))
> 1311 goto out;
> 1312 if (dst_start < dst_vma->vm_start ||
> 1313 dst_start + len > dst_vma->vm_end)
> 1314 goto out;
> 1315
> 1316 err = validate_remap_areas(src_vma, dst_vma);
> 1317 if (err)
> 1318 goto out;
> 1319
> 1320 for (src_addr = src_start, dst_addr = dst_start;
> 1321 src_addr < src_start + len;) {
> 1322 spinlock_t *ptl;
> 1323 pmd_t dst_pmdval;
> 1324 unsigned long step_size;
> 1325
> 1326 BUG_ON(dst_addr >= dst_start + len);
> 1327 /*
> 1328 * Below works because anonymous area would not have a
> 1329 * transparent huge PUD. If file-backed support is added,
> 1330 * that case would need to be handled here.
> 1331 */
> 1332 src_pmd = mm_find_pmd(src_mm, src_addr);
> 1333 if (unlikely(!src_pmd)) {
> 1334 if (!(mode & UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES)) {
> 1335 err = -ENOENT;
> 1336 break;
> 1337 }
> 1338 src_pmd = mm_alloc_pmd(src_mm, src_addr);
> 1339 if (unlikely(!src_pmd)) {
> 1340 err = -ENOMEM;
> 1341 break;
> 1342 }
> 1343 }
> 1344 dst_pmd = mm_alloc_pmd(dst_mm, dst_addr);
> 1345 if (unlikely(!dst_pmd)) {
> 1346 err = -ENOMEM;
> 1347 break;
> 1348 }
> 1349
> 1350 dst_pmdval = pmdp_get_lockless(dst_pmd);
> 1351 /*
> 1352 * If the dst_pmd is mapped as THP don't override it and just
> 1353 * be strict. If dst_pmd changes into TPH after this check, the
> 1354 * remap_pages_huge_pmd() will detect the change and retry
> 1355 * while remap_pages_pte() will detect the change and fail.
> 1356 */
> 1357 if (unlikely(pmd_trans_huge(dst_pmdval))) {
> 1358 err = -EEXIST;
> 1359 break;
> 1360 }
> 1361
> 1362 ptl = pmd_trans_huge_lock(src_pmd, src_vma);
> 1363 if (ptl) {
> 1364 if (pmd_devmap(*src_pmd)) {
> 1365 spin_unlock(ptl);
> 1366 err = -ENOENT;
> 1367 break;
> 1368 }
> 1369
> 1370 /*
> 1371 * Check if we can move the pmd without
> 1372 * splitting it. First check the address
> 1373 * alignment to be the same in src/dst. These
> 1374 * checks don't actually need the PT lock but
> 1375 * it's good to do it here to optimize this
> 1376 * block away at build time if
> 1377 * CONFIG_TRANSPARENT_HUGEPAGE is not set.
> 1378 */
> 1379 if ((src_addr & ~HPAGE_PMD_MASK) || (dst_addr & ~HPAGE_PMD_MASK) ||
> > 1380 src_start + len - src_addr < HPAGE_PMD_SIZE || !pmd_none(dst_pmdval)) {
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
next prev parent reply other threads:[~2023-10-19 21:55 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-09 6:42 [PATCH v3 0/3] userfaultfd move option Suren Baghdasaryan
2023-10-09 6:42 ` [PATCH v3 1/3] mm/rmap: support move to different root anon_vma in folio_move_anon_rmap() Suren Baghdasaryan
2023-10-12 22:01 ` Peter Xu
2023-10-13 8:04 ` David Hildenbrand
2023-10-19 15:19 ` Suren Baghdasaryan
2023-10-09 6:42 ` [PATCH v3 2/3] userfaultfd: UFFDIO_MOVE uABI Suren Baghdasaryan
2023-10-09 14:38 ` David Hildenbrand
2023-10-09 16:21 ` Suren Baghdasaryan
2023-10-09 16:23 ` David Hildenbrand
2023-10-09 16:29 ` Lokesh Gidra
2023-10-09 17:56 ` Lokesh Gidra
2023-10-10 1:49 ` Suren Baghdasaryan
2023-10-12 20:11 ` Peter Xu
2023-10-13 9:56 ` David Hildenbrand
2023-10-13 16:08 ` Peter Xu
2023-10-13 16:49 ` Lokesh Gidra
2023-10-13 17:05 ` Peter Xu
2023-10-16 18:01 ` David Hildenbrand
2023-10-16 19:01 ` Peter Xu
2023-10-17 15:55 ` David Hildenbrand
2023-10-17 18:59 ` Peter Xu
2023-10-19 15:41 ` David Hildenbrand
2023-10-19 19:53 ` Peter Xu
2023-10-19 20:02 ` Suren Baghdasaryan
2023-10-19 20:43 ` Peter Xu
2023-10-20 10:02 ` David Hildenbrand
2023-10-20 14:09 ` Suren Baghdasaryan
2023-10-20 17:16 ` David Hildenbrand
2023-10-22 15:46 ` Peter Xu
2023-10-23 12:03 ` David Hildenbrand
2023-10-23 16:36 ` David Hildenbrand
2023-10-23 17:33 ` Suren Baghdasaryan
2023-10-19 21:45 ` Suren Baghdasaryan
2023-10-12 21:59 ` Peter Xu
2023-10-19 21:24 ` Suren Baghdasaryan
2023-10-22 17:01 ` Peter Xu
2023-10-23 17:43 ` Suren Baghdasaryan
2023-10-23 18:37 ` Peter Xu
2023-10-23 19:01 ` Suren Baghdasaryan
2023-10-17 19:39 ` kernel test robot
2023-10-19 21:55 ` Suren Baghdasaryan [this message]
2023-10-23 12:29 ` David Hildenbrand
2023-10-23 15:53 ` David Hildenbrand
2023-10-23 19:00 ` Suren Baghdasaryan
2023-10-23 18:56 ` Suren Baghdasaryan
2023-10-24 14:27 ` David Hildenbrand
2023-10-24 14:36 ` Suren Baghdasaryan
2023-10-09 6:42 ` [PATCH v3 3/3] selftests/mm: add UFFDIO_MOVE ioctl test Suren Baghdasaryan
2023-10-12 22:29 ` Peter Xu
2023-10-19 15:43 ` Suren Baghdasaryan
2023-10-19 17:29 ` Axel Rasmussen
2023-10-19 19:33 ` Peter Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJuCfpGJqzuJqfH42=f5LXxkf4uCNmUUh2zXyzodczP29O6_hQ@mail.gmail.com' \
--to=surenb@google.com \
--cc=Liam.Howlett@oracle.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=axelrasmussen@google.com \
--cc=bgeffon@google.com \
--cc=brauner@kernel.org \
--cc=david@redhat.com \
--cc=hughd@google.com \
--cc=jannh@google.com \
--cc=jdduke@google.com \
--cc=kaleshsingh@google.com \
--cc=kernel-team@android.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
--cc=lokeshgidra@google.com \
--cc=mhocko@suse.com \
--cc=ngeoffray@google.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=peterx@redhat.com \
--cc=rppt@kernel.org \
--cc=shuah@kernel.org \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
--cc=zhangpeng362@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).