Re: [PATCH] mm/zsmalloc: Replace bit spinlock and get_cpu_var() usage.

[Mike Galbraith <efault@gmx.de>]

On Tue, 2021-09-28 at 15:47 -0700, Andrew Morton wrote:
> On Tue, 28 Sep 2021 10:44:19 +0200 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>
> > From: Mike Galbraith <umgwanakikbuti@gmail.com>
> >
> > For efficiency reasons, zsmalloc is using a slim `handle'. The value is
> > the address of a memory allocation of 4 or 8 bytes depending on the size
> > of the long data type. The lowest bit in that allocated memory is used
> > as a bit spin lock.
> > The usage of the bit spin lock is problematic because with the bit spin
> > lock held zsmalloc acquires a rwlock_t and spinlock_t which are both
> > sleeping locks on PREEMPT_RT and therefore must not be acquired with
> > disabled preemption.
> >
> > Extend the handle to struct zsmalloc_handle which holds the old handle as
> > addr and a spinlock_t which replaces the bit spinlock. Replace all the
> > wrapper functions accordingly.
> >
> > The usage of get_cpu_var() in zs_map_object() is problematic because
> > it disables preemption and makes it impossible to acquire any sleeping
> > lock on PREEMPT_RT such as a spinlock_t.
> > Replace the get_cpu_var() usage with a local_lock_t which is embedded
> > struct mapping_area. It ensures that the access the struct is
> > synchronized against all users on the same CPU.
> >
> > This survived LTP testing.
>
> Rather nasty with all the ifdefs and two different locking approaches
> to be tested.  What would be the impact of simply switching to the new
> scheme for all configs?
>
> Which is identical to asking "what is the impact of switching to the new
> scheme for PREEMPT_RT"!  Which is I think an important thing for the
> changelog to address?

Good questions both, for which I have no answers.  The problematic bit
spinlock going away would certainly be preferred if deemed acceptable.
I frankly doubt either it or zram would be missed were they to instead
be disabled for RT configs.  I certainly have no use for it, making it
functional was a simple case of boy meets bug, and annoys it to death.

	-Mike