lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 20 Jun 2019 17:42:02 +0000
From:   Rik van Riel <riel@...com>
To:     Song Liu <songliubraving@...com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "matthew.wilcox@...cle.com" <matthew.wilcox@...cle.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Kernel Team" <Kernel-team@...com>,
        "william.kucharski@...cle.com" <william.kucharski@...cle.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
Subject: Re: [PATCH v4 6/6] mm,thp: avoid writes to file with THP in pagecache

On Thu, 2019-06-20 at 10:27 -0700, Song Liu wrote:

> +++ b/mm/mmap.c
> @@ -3088,6 +3088,18 @@ int vm_brk(unsigned long addr, unsigned long
> len)
>  }
>  EXPORT_SYMBOL(vm_brk);
>  
> +static inline void release_file_thp(struct vm_area_struct *vma)
> +{
> +#ifdef CONFIG_READ_ONLY_THP_FOR_FS
> +	struct file *file = vma->vm_file;
> +
> +	if (file && (vma->vm_flags & VM_DENYWRITE) &&
> +	    atomic_read(&file_inode(file)->i_writecount) == 0 &&
> +	    filemap_nr_thps(file_inode(file)->i_mapping))
> +		truncate_pagecache(file_inode(file), 0);
> +#endif
> +}
> +
>  /* Release all mmaps. */
>  void exit_mmap(struct mm_struct *mm)
>  {
> @@ -3153,6 +3165,8 @@ void exit_mmap(struct mm_struct *mm)
>  	while (vma) {
>  		if (vma->vm_flags & VM_ACCOUNT)
>  			nr_accounted += vma_pages(vma);
> +
> +		release_file_thp(vma);
>  		vma = remove_vma(vma);
>  	}
>  	vm_unacct_memory(nr_accounted);

I like how you make the file accessible again to other
users, but am somewhat unsure about the mechanism used.

First, if multiple processes have the same file mmapped,
do you really want to blow away the page cache?

Secondly, by hooking into exit_mmap, you miss making
files writable again that get unmapped through munmap.

Would it be better to blow away the page cache when
the last mmap user unmaps it?

The page->mapping->i_mmap interval tree will be empty
when nobody has the file mmap()d.

Alternatively, open() could check whether the file is
currently mmaped, and blow away the page cache then.
That would leave the page cache intact if the same file 
gets execve()d several times in a row without any writes
in-between, which seems like it might be a relatively
common case.



Powered by blists - more mailing lists