Re: [PATCH 8/8] zsmalloc: replace get_cpu_var with local_lock

[Davidlohr Bueso <dave@stgolabs.net>]

On Wed, 10 Nov 2021, Minchan Kim wrote:

>From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
>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.

On a similar note (and sorry for hijacking the thread) I've been looking
at the remaining users of get_cpu_light() in a hope to see them upstreamed
and removed out-of-tree now that we have local locks.

There are six, and afaict, they can be addressed with either using local
locks:

1. netif_rx. We can add a local_lock_t to softnet_data which is the pcpu
data strucutre used by enqueue_to_backlog(). Then replace both preempt_disable
and get_cpu with local_lock(&softnet_data.lock).

2. blk-mq. Such scenarios rely on implicitly disabling preemption to guarantee
running __blk_mq_run_hw_queue() on the right CPU. But we can use a local lock
for synchronous hw queue runs, thus adding a local_lock_t to struct blk_mq_hw_ctx.

3. raid5. We can add a local_lock_t to struct raid5_percpu.

4. scsi/fcoe. There are 3 things here to consider: tx stats management,
fcoe_percpu_s and the exchange manager pool. For the first two adding
a local_lock_t to fc_stats and fcoe_percpu_s should do it, but we would
have to do a migrate_disable() for pool case in fc_exch_em_alloc() which
yes is ugly... pool->lock is already there.

... or flat-out migrate_disabling when the per-CPU data structure already
has a spinlock it acquires anyway, which will do the serialization:

5. vmalloc. Since we already have a vmap_block_queue.lock

6. sunrpc. Since we already have a pool->sp_lock.

I've got patches for these but perhaps I'm missing a fundamental reason as
to why these are still out-of-tree and get_cpu()-light family is still around.
For example, direct migrate_disable() calls are frowned upon and could well
be unacceptable - albeit it's recent user growth upstream.

Thoughts?

Thanks,
Davidlohr