linux-kernel - Re: [PATCH v6 4/6] mm/swapcache: support to handle the exceptional entries in swapcache

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 17 Jun 2020 05:17:17 -0700
From:   Matthew Wilcox <willy@...radead.org>
To:     js1304@...il.com
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Johannes Weiner <hannes@...xchg.org>,
        Michal Hocko <mhocko@...nel.org>,
        Hugh Dickins <hughd@...gle.com>,
        Minchan Kim <minchan@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Mel Gorman <mgorman@...hsingularity.net>, kernel-team@....com,
        Joonsoo Kim <iamjoonsoo.kim@....com>
Subject: Re: [PATCH v6 4/6] mm/swapcache: support to handle the exceptional
 entries in swapcache

On Wed, Jun 17, 2020 at 02:26:21PM +0900, js1304@...il.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@....com>
> 
> Swapcache doesn't handle the exceptional entries since there is no case

Don't call them exceptional entries.

The radix tree has/had the concecpt of exceptional entries.  The swapcache
doesn't use the radix tree any more, it uses the XArray.  The XArray
has value entries.

But you shouldn't call them value entries either; that's an XArray
concept.  The swap cache and indeed page cache use value entries to
implement shadow entries (they're also used to implement dax entries and
swap entries in the page cache).  So just call them shadow entries here.

I know there are still places which use the term 'nrexceptional' in
the kernel.  I just haven't got round to replacing them yet.  Please
don't add more.

> +void clear_shadow_from_swap_cache(int type, unsigned long begin,
> +				unsigned long end)
> +{
> +	unsigned long curr;
> +	void *old;
> +	swp_entry_t entry = swp_entry(type, begin);
> +	struct address_space *address_space = swap_address_space(entry);
> +	XA_STATE(xas, &address_space->i_pages, begin);
> +
> +retry:
> +	xa_lock_irq(&address_space->i_pages);
> +	for (curr = begin; curr <= end; curr++) {
> +		entry = swp_entry(type, curr);
> +		if (swap_address_space(entry) != address_space) {
> +			xa_unlock_irq(&address_space->i_pages);
> +			address_space = swap_address_space(entry);
> +			begin = curr;
> +			xas_set(&xas, begin);
> +			goto retry;
> +		}
> +
> +		old = xas_load(&xas);
> +		if (!xa_is_value(old))
> +			continue;
> +		xas_store(&xas, NULL);
> +		address_space->nrexceptional--;
> +		xas_next(&xas);
> +	}
> +	xa_unlock_irq(&address_space->i_pages);
> +}

This is a very clunky loop.  I'm not sure it's even right, given that
you change address space without changing the xas's address space.  How
about this?

	for (;;) {
		XA_STATE(xas, &address_space->i_pages, begin);
		unsigned long nr_shadows = 0;

		xas_lock_irq(&xas);
		xas_for_each(&xas, entry, end) {
			if (!xa_is_value(entry))
				continue;
			xas_store(&xas, NULL);
			nr_shadows++;
		}
		address_space->nr_exceptionals -= nr_shadows;
		xas_unlock_irq(&xas);

		if (xas.xa_index >= end)
			break;
		entry = swp_entry(type, xas.xa_index);
		address_space = swap_address_space(entry);
	}