linux-kernel - Re: [PATCH v4 1/5] mm: add mkwrite param to vm_insert

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 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 22 Jul 2017 09:21:31 -0700
From:   Dan Williams <dan.j.williams@...il.com>
To:     Ross Zwisler <ross.zwisler@...ux.intel.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jan Kara <jack@...e.cz>, linux-doc@...r.kernel.org,
        David Airlie <airlied@...ux.ie>,
        Dave Chinner <david@...morbit.com>,
        dri-devel@...ts.freedesktop.org, linux-mm <linux-mm@...ck.org>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Patrik Jakobsson <patrik.r.jakobsson@...il.com>,
        Christoph Hellwig <hch@....de>,
        linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
        Joonyoung Shim <jy0922.shim@...sung.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Tomi Valkeinen <tomi.valkeinen@...com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        ext4 hackers <linux-ext4@...r.kernel.org>,
        Matthew Wilcox <mawilcox@...rosoft.com>,
        linux-arm-msm@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Inki Dae <inki.dae@...sung.com>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "Theodore Ts'o" <tytso@....edu>, Jonathan Corbet <corbet@....net>,
        Seung-Woo Kim <sw0312.kim@...sung.com>,
        linux-xfs@...r.kernel.org, Rob Clark <robdclark@...il.com>,
        Kukjin Kim <kgene@...nel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        freedreno@...ts.freedesktop.org
Subject: Re: [PATCH v4 1/5] mm: add mkwrite param to vm_insert_mixed()

On Fri, Jul 21, 2017 at 3:39 PM, Ross Zwisler
<ross.zwisler@...ux.intel.com> wrote:
> To be able to use the common 4k zero page in DAX we need to have our PTE
> fault path look more like our PMD fault path where a PTE entry can be
> marked as dirty and writeable as it is first inserted, rather than waiting
> for a follow-up dax_pfn_mkwrite() => finish_mkwrite_fault() call.
>
> Right now we can rely on having a dax_pfn_mkwrite() call because we can
> distinguish between these two cases in do_wp_page():
>
>         case 1: 4k zero page => writable DAX storage
>         case 2: read-only DAX storage => writeable DAX storage
>
> This distinction is made by via vm_normal_page().  vm_normal_page() returns
> false for the common 4k zero page, though, just as it does for DAX ptes.
> Instead of special casing the DAX + 4k zero page case, we will simplify our
> DAX PTE page fault sequence so that it matches our DAX PMD sequence, and
> get rid of the dax_pfn_mkwrite() helper.  We will instead use
> dax_iomap_fault() to handle write-protection faults.
>
> This means that insert_pfn() needs to follow the lead of insert_pfn_pmd()
> and allow us to pass in a 'mkwrite' flag.  If 'mkwrite' is set insert_pfn()
> will do the work that was previously done by wp_page_reuse() as part of the
> dax_pfn_mkwrite() call path.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@...ux.intel.com>
> ---
>  drivers/dax/device.c                    |  2 +-
>  drivers/gpu/drm/exynos/exynos_drm_gem.c |  3 ++-
>  drivers/gpu/drm/gma500/framebuffer.c    |  2 +-
>  drivers/gpu/drm/msm/msm_gem.c           |  3 ++-
>  drivers/gpu/drm/omapdrm/omap_gem.c      |  6 ++++--
>  drivers/gpu/drm/ttm/ttm_bo_vm.c         |  2 +-
>  fs/dax.c                                |  2 +-
>  include/linux/mm.h                      |  2 +-
>  mm/memory.c                             | 27 +++++++++++++++++++++------
>  9 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index e9f3b3e..3973521 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -273,7 +273,7 @@ static int __dev_dax_pte_fault(struct dev_dax *dev_dax, struct vm_fault *vmf)
>
>         pfn = phys_to_pfn_t(phys, dax_region->pfn_flags);
>
> -       rc = vm_insert_mixed(vmf->vma, vmf->address, pfn);
> +       rc = vm_insert_mixed(vmf->vma, vmf->address, pfn, false);

Ugh, I generally find bool flags unreadable. They place a tax on
jumping to function definition to recall what true and false mean. If
we want to go this 'add an argument' route can we at least add an enum
like:

enum {
    PTE_MKDIRTY,
    PTE_MKCLEAN,
};

...to differentiate the two cases?