Re: [PATCH v10 01/19] zram: sleepable entry locking

[Sebastian Andrzej Siewior]

On 2025-03-03 11:03:10 [+0900], Sergey Senozhatsky wrote:
…
> Having a per-entry mutex (or, for instance, a rw-semaphore)
> significantly increases sizeof() of each entry and hence the
> meta table.  Therefore entry locking returns back to bit
> locking, as before, however, this time also preempt-rt friendly,
> because if waits-on-bit instead of spinning-on-bit.  Lock owners
          of ?
> are also now permitted to schedule, which is a first step on the
> path of making zram non-atomic.
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
…
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -58,19 +58,56 @@ static void zram_free_page(struct zram *zram, size_t index);
>  static int zram_read_from_zspool(struct zram *zram, struct page *page,
>  				 u32 index);
>  
> -static int zram_slot_trylock(struct zram *zram, u32 index)
> +#define slot_dep_map(zram, index) (&(zram)->table[(index)].dep_map)
> +
> +static void zram_slot_lock_init(struct zram *zram, u32 index)
> +{
> +	static struct lock_class_key __key;
> +
> +	lockdep_init_map(slot_dep_map(zram, index), "zram->table[index].lock",
> +			 &__key, 0);
> +}
…
> +static __must_check bool zram_slot_trylock(struct zram *zram, u32 index)
>  {
> -	return spin_trylock(&zram->table[index].lock);
> +	unsigned long *lock = &zram->table[index].flags;
> +
> +	if (!test_and_set_bit_lock(ZRAM_ENTRY_LOCK, lock)) {
> +		mutex_acquire(slot_dep_map(zram, index), 0, 1, _RET_IP_);
> +		lock_acquired(slot_dep_map(zram, index), _RET_IP_);
> +		return true;
> +	}
> +
> +	return false;
>  }
>  
>  static void zram_slot_lock(struct zram *zram, u32 index)
>  {
> -	spin_lock(&zram->table[index].lock);
> +	unsigned long *lock = &zram->table[index].flags;
> +
> +	mutex_acquire(slot_dep_map(zram, index), 0, 0, _RET_IP_);
> +	wait_on_bit_lock(lock, ZRAM_ENTRY_LOCK, TASK_UNINTERRUPTIBLE);
> +	lock_acquired(slot_dep_map(zram, index), _RET_IP_);
>  }

Looking at this, is there a special need to have a lockdep map per
table? Wouldn't it be enough to have one per zram? Logically you use the
same __key so I don't think it makes much of a difference but you could
lower the memory usage a bit by having less of those structs per zram.

Sebastian