linux-kernel - Re: [PATCH v12 00/10] enable bs

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: <20240820091759.vogo5uxaldvik2u2@quentin>
Date: Tue, 20 Aug 2024 09:17:59 +0000
From: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
To: David Howells <dhowells@...hat.com>
Cc: brauner@...nel.org, akpm@...ux-foundation.org, chandan.babu@...cle.com,
	linux-fsdevel@...r.kernel.org, djwong@...nel.org, hare@...e.de,
	gost.dev@...sung.com, linux-xfs@...r.kernel.org, hch@....de,
	david@...morbit.com, Zi Yan <ziy@...dia.com>,
	yang@...amperecomputing.com, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, willy@...radead.org, john.g.garry@...cle.com,
	cl@...amperecomputing.com, p.raghav@...sung.com, mcgrof@...nel.org,
	ryan.roberts@....com
Subject: Re: [PATCH v12 00/10] enable bs > ps in XFS

On Mon, Aug 19, 2024 at 07:40:44PM +0100, David Howells wrote:
> Pankaj Raghav (Samsung) <kernel@...kajraghav.com> wrote:
> 
> > I tried this code on XFS, and it is working as expected (I am getting
> > xxxx).
> 
> XFS doesn't try to use mapping_set_release_always().

Thanks David for digging deep. It is indeed a bug in this patchset
(PATCH 1). I think I overlooked the way we MASK the folio order bits
when we changed it sometime back. 

But still I don't know why AS_RELEASE_ALWAYS is being cleared because it
is in BIT 6, and existing bug should not affect BIT 6.

The following triggers an ASSERT failure.

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 0fcf235e5023..35961d73d54a 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -88,9 +88,13 @@ xfs_inode_alloc(
 
        /* VFS doesn't initialise i_mode! */
        VFS_I(ip)->i_mode = 0;
+       mapping_set_unevictable(VFS_I(ip)->i_mapping);
        mapping_set_folio_min_order(VFS_I(ip)->i_mapping,
                                    M_IGEO(mp)->min_folio_order);
 
+       ASSERT(mapping_unevictable(VFS_I(ip)->i_mapping) == 1);
+
+       mapping_clear_unevictable(VFS_I(ip)->i_mapping);
        XFS_STATS_INC(mp, vn_active);
        ASSERT(atomic_read(&ip->i_pincount) == 0);
        ASSERT(ip->i_ino == 0);

The patch that fixes this is:

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 61a7649d86e5..5e245b8dcfd6 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -217,6 +217,7 @@ enum mapping_flags {
 #define AS_FOLIO_ORDER_MASK     ((1u << AS_FOLIO_ORDER_BITS) - 1)
 #define AS_FOLIO_ORDER_MIN_MASK (AS_FOLIO_ORDER_MASK << AS_FOLIO_ORDER_MIN)
 #define AS_FOLIO_ORDER_MAX_MASK (AS_FOLIO_ORDER_MASK << AS_FOLIO_ORDER_MAX)
+#define AS_FOLIO_ORDER_MIN_MAX_MASK (AS_FOLIO_ORDER_MIN_MASK | AS_FOLIO_ORDER_MAX_MASK)
 
 /**
  * mapping_set_error - record a writeback error in the address_space
@@ -418,7 +419,7 @@ static inline void mapping_set_folio_order_range(struct address_space *mapping,
        if (max < min)
                max = min;
 
-       mapping->flags = (mapping->flags & ~AS_FOLIO_ORDER_MASK) |
+       mapping->flags = (mapping->flags & ~AS_FOLIO_ORDER_MIN_MAX_MASK) |
                (min << AS_FOLIO_ORDER_MIN) | (max << AS_FOLIO_ORDER_MAX);
 }
 
Could you try this patch and see if it fixes it by any chance?

--
Pankaj