Re: [PATCH RFC v2 1/2] mm: Don't skip swap entry even if zap_details specified

[Peter Xu <peterx@xxxxxxxxxx>]

On Mon, Jan 24, 2022 at 04:54:29PM +0800, Peter Xu wrote:
> I have a quick idea on reproducer now (perhaps file size shrinking on private
> pages being swapped out), I'll try to write a real reproducer and update later.

Here's the reproducer..

===8<===
#define _GNU_SOURCE         /* See feature_test_macros(7) */
#include <stdio.h>
#include <assert.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/types.h>

int page_size;
int shmem_fd;
char *buffer;

void main(void)
{
	int ret;
	char val;

	page_size = getpagesize();
	shmem_fd = memfd_create("test", 0);
	assert(shmem_fd >= 0);

	ret = ftruncate(shmem_fd, page_size * 2);
	assert(ret == 0);

	buffer = mmap(NULL, page_size * 2, PROT_READ | PROT_WRITE,
			MAP_PRIVATE, shmem_fd, 0);
	assert(buffer != MAP_FAILED);

	/* Write private page, swap it out */
	buffer[page_size] = 1;
	madvise(buffer, page_size * 2, MADV_PAGEOUT);

	/* This should drop private buffer[page_size] already */
	ret = ftruncate(shmem_fd, page_size);
	assert(ret == 0);
	/* Recover the size */
	ret = ftruncate(shmem_fd, page_size * 2);
	assert(ret == 0);

	/* Re-read the data, it should be all zero */
	val = buffer[page_size];
	if (val == 0)
		printf("Good\n");
	else
		printf("BUG\n");
}
===8<===

I'm looking for a correct Fixes for the 1st patch, but afaict it seems not
right even in the initial git commit, which traces back to the "history" git
repo of your commit:

dd9fd0e03de6 ("[PATCH] rmap: nonlinear truncation", 2004-04-17)

Where we have:

		/*
		 * If details->check_mapping, we leave swap entries;
		 * if details->nonlinear_vma, we leave file entries.
		 */
		if (unlikely(details))
			continue;
		if (!pte_file(ptent))
			free_swap_and_cache(pte_to_swp_entry(ptent));

As I don't know how it could be right if the user only specified
details->nonlinear_vma but keeping details->check_mapping==NULL.. there, and
there're definitely callers for that, e.g.:

static long madvise_dontneed(struct vm_area_struct * vma,
			     unsigned long start, unsigned long end)
{
	if ((vma->vm_flags & VM_LOCKED) || is_vm_hugetlb_page(vma))
		return -EINVAL;

	if (unlikely(vma->vm_flags & VM_NONLINEAR)) {
		struct zap_details details = {
			.nonlinear_vma = vma,
			.last_index = ULONG_MAX,
		};
		zap_page_range(vma, start, end - start, &details);
	} else
		zap_page_range(vma, start, end - start, NULL);
	return 0;
}

So I plan to add the Fixes to the initial git commit, 1da177e4c3f4
("Linux-2.6.12-rc2", 2005-04-16).

Does it sound right to you?

Thanks,

--
Peter Xu