From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29B73C433FE for ; Tue, 22 Nov 2022 08:54:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8F6106B0074; Tue, 22 Nov 2022 03:54:01 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8A6006B0075; Tue, 22 Nov 2022 03:54:01 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 794CF8E0001; Tue, 22 Nov 2022 03:54:01 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 6BFDA6B0074 for ; Tue, 22 Nov 2022 03:54:01 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 45EEC1407EB for ; Tue, 22 Nov 2022 08:54:01 +0000 (UTC) X-FDA: 80160465882.03.08CF307 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by imf13.hostedemail.com (Postfix) with ESMTP id AEB582000B for ; Tue, 22 Nov 2022 08:54:00 +0000 (UTC) Received: from [192.168.10.9] (unknown [39.45.241.105]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: usama.anjum) by madras.collabora.co.uk (Postfix) with ESMTPSA id AE3616602A89; Tue, 22 Nov 2022 08:53:56 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1669107239; bh=8ZEvrn+4mbqku0WNRgI9tdJCkGeYKUqf7aXJXCGXOEc=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=H/LSoreWTskXcbuNcSY+biZE1CZrnl06I5k9aU13bMl3fHDzbhrGsyrbmPWABc3o0 OLC6tUMM9GqwP6qic+HevPlAXgNLCVtDICcvIEw8NZLrfDcg5uISB7/LrHoerMRM9o NMjmLIcbpk6Bm3nU+6qRwtukRbsFGsjaiQ/hdJg1OKuYkqTUllSf4Iiac8R0akBWjW RIpgIQTSRH9T5P/2JxX44I2R9U3NjzWOcYn611fjRk4Yv5Q3B1UEWnWC48WtLXpkie o5GdgFLi/pNApb2n3B0cSq62GewRdHIEf+8ZHJMlh8pGMlGG8pVNLvDk8fPja36uNZ KgMRUUXnYr0aA== Message-ID: Date: Tue, 22 Nov 2022 13:53:52 +0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Cc: Muhammad Usama Anjum , Mel Gorman , Peter Xu , Andrei Vagin , kernel@collabora.com, stable@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: set the vma flags dirty before testing if it is mergeable Content-Language: en-US To: David Hildenbrand , Andrew Morton , Cyrill Gorcunov References: <20221122082442.1938606-1-usama.anjum@collabora.com> <4ebdbc0f-6352-5020-3f74-94e6c3743a1d@collabora.com> From: Muhammad Usama Anjum In-Reply-To: <4ebdbc0f-6352-5020-3f74-94e6c3743a1d@collabora.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b="H/LSoreW"; spf=pass (imf13.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.172 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=none) header.from=collabora.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669107241; a=rsa-sha256; cv=none; b=BJu0eBYKJkTUOf87BsXYDVzdOQHCpV0ttje5fJYqT/sBCDUjmjJoiYF+ooNxvuXDJmXs9L BbKppz8JfJ0YFFdpJVM6xGAEM1b6IGvUVSG9ZpSGc6wgREmVcD1dRInJ0F6h9SNAEuCC9w fe3t+15fbPWiHvK558m6CTdF3+P39pg= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669107240; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=nvmSPnX7HvAKZjz6zBFSwiNXQBCXYenZcJxUF3xWlqY=; b=tNTan03uTse7Vxi9Iu6eRSEw9mszcPNDsgllzgYw+/eWEaJk/ZBNgXoJQ73Yy3lsw+XgA9 JWquN5+6tZs1mWDK5O0jcSMvGaS/QV+taGPSe/Pi0kWQ74SnQ/jRbgGeVQmspyEOogclQl yXObBD2b81/AgWl51jcL1o/jylIe55Q= X-Stat-Signature: 6gg7n71npjthshiutjxi8pazfg9mh7z8 X-Rspamd-Server: rspam08 X-Rspam-User: Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b="H/LSoreW"; spf=pass (imf13.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.172 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=none) header.from=collabora.com X-Rspamd-Queue-Id: AEB582000B X-HE-Tag: 1669107240-639000 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 11/22/22 1:49 PM, Muhammad Usama Anjum wrote: > On 11/22/22 1:36 PM, David Hildenbrand wrote: >> On 22.11.22 09:24, Muhammad Usama Anjum wrote: >>> The VM_SOFTDIRTY should be set in the vma flags to be tested if new >>> allocation should be merged in previous vma or not. With this patch, >>> the new allocations are merged in the previous VMAs. >>> >>> I've tested it by reverting the commit 34228d473efe ("mm: ignore >>> VM_SOFTDIRTY on VMA merging") and after adding this following patch, >>> I'm seeing that all the new allocations done through mmap() are merged >>> in the previous VMAs. The number of VMAs doesn't increase drastically >>> which had contributed to the crash of gimp. If I run the same test after >>> reverting and not including this patch, the number of VMAs keep on >>> increasing with every mmap() syscall which proves this patch. >>> >>> The commit 34228d473efe ("mm: ignore VM_SOFTDIRTY on VMA merging") >>> seems like a workaround. But it lets the soft-dirty and non-soft-dirty >>> VMA to get merged. It helps in avoiding the creation of too many VMAs. >>> But it creates the problem while adding the feature of clearing the >>> soft-dirty status of only a part of the memory region. >>> >>> Cc: >>> Fixes: d9104d1ca966 ("mm: track vma changes with VM_SOFTDIRTY bit") >>> Signed-off-by: Muhammad Usama Anjum >>> --- >>> We need more testing of this patch. >>> >>> While implementing clear soft-dirty bit for a range of address space, I'm >>> facing an issue. The non-soft dirty VMA gets merged sometimes with the soft >>> dirty VMA. Thus the non-soft dirty VMA become dirty which is undesirable. >>> When discussed with the some other developers they consider it the >>> regression. Why the non-soft dirty page should appear as soft dirty when it >>> isn't soft dirty in reality? I agree with them. Should we revert >>> 34228d473efe or find a workaround in the IOCTL? >>> >>> * Revert may cause the VMAs to expand in uncontrollable situation where the >>> soft dirty bit of a lot of memory regions or the whole address space is >>> being cleared again and again. AFAIK normal process must either be only >>> clearing a few memory regions. So the applications should be okay. There is >>> still chance of regressions if some applications are already using the >>> soft-dirty bit. I'm not sure how to test it. >>> >>> * Add a flag in the IOCTL to ignore the dirtiness of VMA. The user will >>> surely lose the functionality to detect reused memory regions. But the >>> extraneous soft-dirty pages would not appear. I'm trying to do this in the >>> patch series [1]. Some discussion is going on that this fails with some >>> mprotect use case [2]. I still need to have a look at the mprotect selftest >>> to see how and why this fails. I think this can be implemented after some >>> more work probably in mprotect side. >>> >>> [1] >>> https://lore.kernel.org/all/20221109102303.851281-1-usama.anjum@collabora.com/ >>> [2] >>> https://lore.kernel.org/all/bfcae708-db21-04b4-0bbe-712badd03071@redhat.com/ >>> --- >>>   mm/mmap.c | 21 +++++++++++---------- >>>   1 file changed, 11 insertions(+), 10 deletions(-) >>> >>> diff --git a/mm/mmap.c b/mm/mmap.c >>> index f9b96b387a6f..6934b8f61fdc 100644 >>> --- a/mm/mmap.c >>> +++ b/mm/mmap.c >>> @@ -1708,6 +1708,15 @@ unsigned long mmap_region(struct file *file, >>> unsigned long addr, >>>           vm_flags |= VM_ACCOUNT; >>>       } >>>   +    /* >>> +     * New (or expanded) vma always get soft dirty status. >>> +     * Otherwise user-space soft-dirty page tracker won't >>> +     * be able to distinguish situation when vma area unmapped, >>> +     * then new mapped in-place (which must be aimed as >>> +     * a completely new data area). >>> +     */ >>> +    vm_flags |= VM_SOFTDIRTY; >>> + >>>       /* >>>        * Can we just expand an old mapping? >>>        */ >>> @@ -1823,15 +1832,6 @@ unsigned long mmap_region(struct file *file, >>> unsigned long addr, >>>       if (file) >>>           uprobe_mmap(vma); >>>   -    /* >>> -     * New (or expanded) vma always get soft dirty status. >>> -     * Otherwise user-space soft-dirty page tracker won't >>> -     * be able to distinguish situation when vma area unmapped, >>> -     * then new mapped in-place (which must be aimed as >>> -     * a completely new data area). >>> -     */ >>> -    vma->vm_flags |= VM_SOFTDIRTY; >>> - >>>       vma_set_page_prot(vma); >> >> vma_set_page_prot(vma) has to be called after adjusting vma->vm_flags. >> >> Did not look into the details here, but that jumped at me. > vma_set_page_prot() also needs to be removed from here as it was being > called after updating the vm_flags. I'll remove it. vma_set_page_prot() was > added in a separate commit 64e455079e1b. I'll send a v2 in a while. Just had another look. vm_flags is being modified just above vma_set_page_prot(). So we don't need to remove it. -- BR, Muhammad Usama Anjum