Skip to content

Commit f60b663

Browse files
xzpeterakpm00
authored andcommitted
mm/selftests: add a test to verify mmap_changing race with -EAGAIN
Add an unit test to verify the recent mmap_changing ABI breakage. Note that I used some tricks here and there to make the test simple, e.g. I abused UFFDIO_MOVE on top of shmem with the fact that I know what I want to test will be even earlier than the vma type check. Rich comments were added to explain trivial details. Before that fix, -EAGAIN would have been written to the copy field most of the time but not always; the test should be able to reliably trigger the outlier case. After the fix, it's written always, the test verifies that making sure corresponding field (e.g. copy.copy for UFFDIO_COPY) is updated. [[email protected]: coding-style cleanups] Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Peter Xu <[email protected]> Cc: Andrea Arcangeli <[email protected]> Cc: Axel Rasmussen <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 4428a35 commit f60b663

File tree

1 file changed

+202
-0
lines changed

1 file changed

+202
-0
lines changed

tools/testing/selftests/mm/uffd-unit-tests.c

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,182 @@ static void uffd_move_pmd_split_test(uffd_test_args_t *targs)
12311231
uffd_move_pmd_handle_fault);
12321232
}
12331233

1234+
static bool
1235+
uffdio_verify_results(const char *name, int ret, int error, long result)
1236+
{
1237+
/*
1238+
* Should always return -1 with errno=EAGAIN, with corresponding
1239+
* result field updated in ioctl() args to be -EAGAIN too
1240+
* (e.g. copy.copy field for UFFDIO_COPY).
1241+
*/
1242+
if (ret != -1) {
1243+
uffd_test_fail("%s should have returned -1", name);
1244+
return false;
1245+
}
1246+
1247+
if (error != EAGAIN) {
1248+
uffd_test_fail("%s should have errno==EAGAIN", name);
1249+
return false;
1250+
}
1251+
1252+
if (result != -EAGAIN) {
1253+
uffd_test_fail("%s should have been updated for -EAGAIN",
1254+
name);
1255+
return false;
1256+
}
1257+
1258+
return true;
1259+
}
1260+
1261+
/*
1262+
* This defines a function to test one ioctl. Note that here "field" can
1263+
* be 1 or anything not -EAGAIN. With that initial value set, we can
1264+
* verify later that it should be updated by kernel (when -EAGAIN
1265+
* returned), by checking whether it is also updated to -EAGAIN.
1266+
*/
1267+
#define DEFINE_MMAP_CHANGING_TEST(name, ioctl_name, field) \
1268+
static bool uffdio_mmap_changing_test_##name(int fd) \
1269+
{ \
1270+
int ret; \
1271+
struct uffdio_##name args = { \
1272+
.field = 1, \
1273+
}; \
1274+
ret = ioctl(fd, ioctl_name, &args); \
1275+
return uffdio_verify_results(#ioctl_name, ret, errno, args.field); \
1276+
}
1277+
1278+
DEFINE_MMAP_CHANGING_TEST(zeropage, UFFDIO_ZEROPAGE, zeropage)
1279+
DEFINE_MMAP_CHANGING_TEST(copy, UFFDIO_COPY, copy)
1280+
DEFINE_MMAP_CHANGING_TEST(move, UFFDIO_MOVE, move)
1281+
DEFINE_MMAP_CHANGING_TEST(poison, UFFDIO_POISON, updated)
1282+
DEFINE_MMAP_CHANGING_TEST(continue, UFFDIO_CONTINUE, mapped)
1283+
1284+
typedef enum {
1285+
/* We actually do not care about any state except UNINTERRUPTIBLE.. */
1286+
THR_STATE_UNKNOWN = 0,
1287+
THR_STATE_UNINTERRUPTIBLE,
1288+
} thread_state;
1289+
1290+
static void sleep_short(void)
1291+
{
1292+
usleep(1000);
1293+
}
1294+
1295+
static thread_state thread_state_get(pid_t tid)
1296+
{
1297+
const char *header = "State:\t";
1298+
char tmp[256], *p, c;
1299+
FILE *fp;
1300+
1301+
snprintf(tmp, sizeof(tmp), "/proc/%d/status", tid);
1302+
fp = fopen(tmp, "r");
1303+
1304+
if (!fp)
1305+
return THR_STATE_UNKNOWN;
1306+
1307+
while (fgets(tmp, sizeof(tmp), fp)) {
1308+
p = strstr(tmp, header);
1309+
if (p) {
1310+
/* For example, "State:\tD (disk sleep)" */
1311+
c = *(p + sizeof(header) - 1);
1312+
return c == 'D' ?
1313+
THR_STATE_UNINTERRUPTIBLE : THR_STATE_UNKNOWN;
1314+
}
1315+
}
1316+
1317+
return THR_STATE_UNKNOWN;
1318+
}
1319+
1320+
static void thread_state_until(pid_t tid, thread_state state)
1321+
{
1322+
thread_state s;
1323+
1324+
do {
1325+
s = thread_state_get(tid);
1326+
sleep_short();
1327+
} while (s != state);
1328+
}
1329+
1330+
static void *uffd_mmap_changing_thread(void *opaque)
1331+
{
1332+
volatile pid_t *pid = opaque;
1333+
int ret;
1334+
1335+
/* Unfortunately, it's only fetch-able from the thread itself.. */
1336+
assert(*pid == 0);
1337+
*pid = syscall(SYS_gettid);
1338+
1339+
/* Inject an event, this will hang solid until the event read */
1340+
ret = madvise(area_dst, page_size, MADV_REMOVE);
1341+
if (ret)
1342+
err("madvise(MADV_REMOVE) failed");
1343+
1344+
return NULL;
1345+
}
1346+
1347+
static void uffd_consume_message(int fd)
1348+
{
1349+
struct uffd_msg msg = { 0 };
1350+
1351+
while (uffd_read_msg(fd, &msg));
1352+
}
1353+
1354+
static void uffd_mmap_changing_test(uffd_test_args_t *targs)
1355+
{
1356+
/*
1357+
* This stores the real PID (which can be different from how tid is
1358+
* defined..) for the child thread, 0 means not initialized.
1359+
*/
1360+
pid_t pid = 0;
1361+
pthread_t tid;
1362+
int ret;
1363+
1364+
if (uffd_register(uffd, area_dst, nr_pages * page_size,
1365+
true, false, false))
1366+
err("uffd_register() failed");
1367+
1368+
/* Create a thread to generate the racy event */
1369+
ret = pthread_create(&tid, NULL, uffd_mmap_changing_thread, &pid);
1370+
if (ret)
1371+
err("pthread_create() failed");
1372+
1373+
/*
1374+
* Wait until the thread setup the pid. Use volatile to make sure
1375+
* it reads from RAM not regs.
1376+
*/
1377+
while (!(volatile pid_t)pid)
1378+
sleep_short();
1379+
1380+
/* Wait until the thread hangs at REMOVE event */
1381+
thread_state_until(pid, THR_STATE_UNINTERRUPTIBLE);
1382+
1383+
if (!uffdio_mmap_changing_test_copy(uffd))
1384+
return;
1385+
1386+
if (!uffdio_mmap_changing_test_zeropage(uffd))
1387+
return;
1388+
1389+
if (!uffdio_mmap_changing_test_move(uffd))
1390+
return;
1391+
1392+
if (!uffdio_mmap_changing_test_poison(uffd))
1393+
return;
1394+
1395+
if (!uffdio_mmap_changing_test_continue(uffd))
1396+
return;
1397+
1398+
/*
1399+
* All succeeded above! Recycle everything. Start by reading the
1400+
* event so as to kick the thread roll again..
1401+
*/
1402+
uffd_consume_message(uffd);
1403+
1404+
ret = pthread_join(tid, NULL);
1405+
assert(ret == 0);
1406+
1407+
uffd_test_pass();
1408+
}
1409+
12341410
static int prevent_hugepages(const char **errmsg)
12351411
{
12361412
/* This should be done before source area is populated */
@@ -1470,6 +1646,32 @@ uffd_test_case_t uffd_tests[] = {
14701646
.mem_targets = MEM_ALL,
14711647
.uffd_feature_required = UFFD_FEATURE_POISON,
14721648
},
1649+
{
1650+
.name = "mmap-changing",
1651+
.uffd_fn = uffd_mmap_changing_test,
1652+
/*
1653+
* There's no point running this test over all mem types as
1654+
* they share the same code paths.
1655+
*
1656+
* Choose shmem for simplicity, because (1) shmem supports
1657+
* MINOR mode to cover UFFDIO_CONTINUE, and (2) shmem is
1658+
* almost always available (unlike hugetlb). Here we
1659+
* abused SHMEM for UFFDIO_MOVE, but the test we want to
1660+
* cover doesn't yet need the correct memory type..
1661+
*/
1662+
.mem_targets = MEM_SHMEM,
1663+
/*
1664+
* Any UFFD_FEATURE_EVENT_* should work to trigger the
1665+
* race logically, but choose the simplest (REMOVE).
1666+
*
1667+
* Meanwhile, since we'll cover quite a few new ioctl()s
1668+
* (CONTINUE, POISON, MOVE), skip this test for old kernels
1669+
* by choosing all of them.
1670+
*/
1671+
.uffd_feature_required = UFFD_FEATURE_EVENT_REMOVE |
1672+
UFFD_FEATURE_MOVE | UFFD_FEATURE_POISON |
1673+
UFFD_FEATURE_MINOR_SHMEM,
1674+
},
14731675
};
14741676

14751677
static void usage(const char *prog)

0 commit comments

Comments
 (0)