Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 24 Jan 2022 19:01:04 +0800
From:   Peter Xu <peterx@redhat.com>
To:     Hugh Dickins <hughd@google.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        David Hildenbrand <david@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Yang Shi <shy828301@gmail.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Andrew Morton <akpm@linux-foundation.org>,
        Alistair Popple <apopple@nvidia.com>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH RFC v2 1/2] mm: Don't skip swap entry even if zap_details
 specified
Message-ID: <Ye6G8F0VOQRGwcXe@xz-m1.local>
References: <20211115134951.85286-1-peterx@redhat.com>
 <20211115134951.85286-2-peterx@redhat.com>
 <9937aaa-d9ab-2839-b0b7-691d85c9141@google.com>
 <Yd7VEXI+/Lu8MVOo@xz-m1.local>
 <Yd7W6ndSPkXQjurY@xz-m1.local>
 <391aa58d-ce84-9d4-d68d-d98a9c533255@google.com>
 <Yek6PaW8fAXY4Bft@xz-m1.local>
 <93dd745c-5e8b-a50-4ec5-b3f3728ad8b@google.com>
 <Ye5pRQNRv53HWmSZ@xz-m1.local>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Ye5pRQNRv53HWmSZ@xz-m1.local>
Precedence: bulk

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