linux-kernel - RE: [PATCH 1/2] fsdax,xfs: fix warning messages at dax_[dis]associate

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: <6386d82011018_c95729485@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Date:   Tue, 29 Nov 2022 20:12:16 -0800
From:   Dan Williams <dan.j.williams@...el.com>
To:     Shiyang Ruan <ruansy.fnst@...itsu.com>,
        <linux-kernel@...r.kernel.org>, <linux-xfs@...r.kernel.org>,
        <nvdimm@...ts.linux.dev>, <linux-fsdevel@...r.kernel.org>
CC:     <djwong@...nel.org>, <david@...morbit.com>,
        <dan.j.williams@...el.com>
Subject: RE: [PATCH 1/2] fsdax,xfs: fix warning messages at
 dax_[dis]associate_entry()

Shiyang Ruan wrote:
> This patch fixes the warning message reported in dax_associate_entry()
> and dax_disassociate_entry().

Can you include the xfstest test number and a snippet of the warning
message.

> 1. reset page->mapping and ->index when refcount counting down to 0.
> 2. set IOMAP_F_SHARED flag when iomap read to allow one dax page to be
> associated more than once for not only write but also read.
> 3. should zero the edge (when not aligned) if srcmap is HOLE or
> UNWRITTEN.
> 4. iterator of two files in dedupe should be executed side by side, not
> nested.
> 5. use xfs_dax_write_iomap_ops for xfs zero and truncate. 

Do these all need to be done at once, or is this 5 patches?

> 
> Signed-off-by: Shiyang Ruan <ruansy.fnst@...itsu.com>
> ---
>  fs/dax.c           | 114 ++++++++++++++++++++++++++-------------------
>  fs/xfs/xfs_iomap.c |   6 +--
>  2 files changed, 69 insertions(+), 51 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 1c6867810cbd..5ea7c0926b7f 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -398,7 +398,7 @@ static void dax_disassociate_entry(void *entry, struct address_space *mapping,
>  		WARN_ON_ONCE(trunc && page_ref_count(page) > 1);
>  		if (dax_mapping_is_cow(page->mapping)) {
>  			/* keep the CoW flag if this page is still shared */
> -			if (page->index-- > 0)
> +			if (page->index-- > 1)

I think this wants either a helper function to make it clear that
->index is being used as a share count, or go ahead and rename that
field in this context with something like:

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 910d880e67eb..1a409288f39d 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -103,7 +103,10 @@ struct page {
                        };
                        /* See page-flags.h for PAGE_MAPPING_FLAGS */
                        struct address_space *mapping;
-                       pgoff_t index;          /* Our offset within mapping. */
+                       union {
+                               pgoff_t index;          /* Our offset within mapping. */
+                               unsigned long share;
+                       };
                        /**
                         * @private: Mapping-private opaque data.
                         * Usually used for buffer_heads if PagePrivate.