Mika Penttilä <mpenttil@xxxxxxxxxx> writes: > Hi Alistair! > > On 12.8.2022 8.22, Alistair Popple wrote: [...] >> + buffer->ptr = mmap(NULL, size, >> + PROT_READ | PROT_WRITE, >> + MAP_PRIVATE | MAP_ANONYMOUS, >> + buffer->fd, 0); >> + ASSERT_NE(buffer->ptr, MAP_FAILED); >> + >> + /* Initialize buffer in system memory. */ >> + for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i) >> + ptr[i] = 0; >> + >> + ASSERT_FALSE(write_cgroup_param(cgroup, "memory.reclaim", 1UL<<30)); >> + >> + /* Fault pages back in from swap as clean pages */ >> + for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i) >> + tmp += ptr[i]; >> + >> + /* Dirty the pte */ >> + for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i) >> + ptr[i] = i; >> + > > The anon pages are quite likely in memory at this point, and dirty in pte. Why would the pte be dirty? I just confirmed using some modified pagemap code that on my system at least this isn't the case. >> + /* >> + * Attempt to migrate memory to device, which should fail because >> + * hopefully some pages are backed by swap storage. >> + */ >> + ASSERT_TRUE(hmm_migrate_sys_to_dev(self->fd, buffer, npages)); > > And pages marked dirty also now. But could you elaborate how and where the above > fails in more detail, couldn't immediately see it... Not if you don't have patch 1 of this series applied. If the trylock_page() in migrate_vma_collect_pmd() succeeds (which it almost always does) it will have cleared the pte without setting PageDirty. So now we have a dirty page without PageDirty set and without a dirty pte. If this page gets swapped back to disk and is still in the swap cache data will be lost because reclaim will see a clean page and won't write it out again. At least that's my understanding - please let me know if you see something that doesn't make sense. >> + >> + ASSERT_FALSE(write_cgroup_param(cgroup, "memory.reclaim", 1UL<<30)); >> + >> + /* Check we still see the updated data after restoring from swap. */ >> + for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i) >> + ASSERT_EQ(ptr[i], i); >> + >> + hmm_buffer_free(buffer); >> + destroy_cgroup(); >> +} >> + >> /* >> * Read anonymous memory multiple times. >> */ > > > --Mika
