linux-ext4 - Re: [PATCH v4 1/2] dax: dax_layout_busy

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]

Message-ID: <6ca60cf6-f1dc-27d5-fb79-7547aea39dfa@redhat.com>
Date:   Fri, 10 Aug 2018 14:52:53 -0500
From:   Eric Sandeen <esandeen@...hat.com>
To:     Ross Zwisler <ross.zwisler@...ux.intel.com>,
        Jan Kara <jack@...e.cz>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Chinner <david@...morbit.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Christoph Hellwig <hch@....de>, linux-nvdimm@...ts.01.org,
        Jeff Moyer <jmoyer@...hat.com>, linux-ext4@...r.kernel.org,
        Lukas Czerner <lczerner@...hat.com>
Subject: Re: [PATCH v4 1/2] dax: dax_layout_busy_page() warn on !exceptional

On 7/10/18 2:10 PM, Ross Zwisler wrote:
> Inodes using DAX should only ever have exceptional entries in their page
> caches.  Make this clear by warning if the iteration in
> dax_layout_busy_page() ever sees a non-exceptional entry, and by adding a
> comment for the pagevec_release() call which only deals with struct page
> pointers.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@...ux.intel.com>
> Reviewed-by: Jan Kara <jack@...e.cz>

Hi Ted, hadn't seem feedback from you on this by the time it gathered reviews,
is this something you plan to merge for realz?  (I see it's on your dev
branch now, just not sure of its permanence at this point.)

Thanks,
-Eric

> ---
>  fs/dax.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 641192808bb6..897b51e41d8f 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -566,7 +566,8 @@ struct page *dax_layout_busy_page(struct address_space *mapping)
>  			if (index >= end)
>  				break;
>  
> -			if (!radix_tree_exceptional_entry(pvec_ent))
> +			if (WARN_ON_ONCE(
> +			     !radix_tree_exceptional_entry(pvec_ent)))
>  				continue;
>  
>  			xa_lock_irq(&mapping->i_pages);
> @@ -578,6 +579,13 @@ struct page *dax_layout_busy_page(struct address_space *mapping)
>  			if (page)
>  				break;
>  		}
> +
> +		/*
> +		 * We don't expect normal struct page entries to exist in our
> +		 * tree, but we keep these pagevec calls so that this code is
> +		 * consistent with the common pattern for handling pagevecs
> +		 * throughout the kernel.
> +		 */
>  		pagevec_remove_exceptionals(&pvec);
>  		pagevec_release(&pvec);
>  		index++;
>