linux-kernel - Re: [PATCH v7 08/61] xarray: Add the xa_lock to the radix_tree

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, 3 Mar 2018 08:57:28 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Jeff Layton <jlayton@...nel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Matthew Wilcox <mawilcox@...rosoft.com>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v7 08/61] xarray: Add the xa_lock to the radix_tree_root

On Sat, Mar 03, 2018 at 09:55:22AM -0500, Jeff Layton wrote:
> On Mon, 2018-02-19 at 11:45 -0800, Matthew Wilcox wrote:
> > From: Matthew Wilcox <mawilcox@...rosoft.com>
> > 
> > This results in no change in structure size on 64-bit x86 as it fits in
> > the padding between the gfp_t and the void *.
> > 
> 
> While the patch itself looks fine, we should take note that this will
> likely increase the size of radix_tree_root on 32-bit arches.
> 
> I don't think that's necessarily a deal breaker, but there are a lot of
> users of radix_tree_root. Many of those users have their own spinlock
> for radix tree accesses, and could be trivially changed to use the
> xa_lock. That would need to be done piecemeal though.
> 
> A less disruptive idea might be to just create some new struct that's a
> spinlock + radix_tree_root, and then use that going forward in the
> xarray conversion. That might be better anyway if you're considering a
> more phased approach for getting this merged.

Well, it's a choice.  If we do:

struct xarray {
	spinlock_t xa_lock;
	struct radix_tree_root root;
};

then the padding on 64-bit turns that into a 24-byte struct.  So do we
spend the extra 4 bytes on 32-bit and have the struct the way we want it
to look from the beginning, or do we spend the extra 8 bytes on 64-bit
and have to redo the struct accessors after the conversions are complete?
I chose option (a), but reasonable people can disagree on that choice.