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

[Sebastian Andrzej Siewior]

On 2026-07-07 18:42:50 [+0900], Sergey Senozhatsky wrote:
> On (26/07/07 11:30), Sebastian Andrzej Siewior wrote:
> > > > >  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.
> > > 
> > > So the idea was to separate lockdep maps and keep them per-device
> > > because various zram devices can be on different IO paths with very
> > > different locking graphs.  E.g. zram0 can be a swap device with all
> > > the locks that swap layer has, while zram1 can be a "normal" block
> > > device mounted with ext4 with all the locks that ext4/vfs bring.
> > 
> > Ach so index will be 0 for zram0 and 1 for zram1? In that case it is
> > different from what I assumed. But you use the same key so I think it
> > will be the same lock from lockdep's point of view. Like it will
> > complain if you do
> > 	zram_slot_lock(zram, 0);
> > 	zram_slot_lock(zram, 1);
> 
> index is PAGE_SIZE granularity "block" of device that holds compressed
> data.  So they go from 0 to zram->disksize / PAGE_SIZE.  Should I use
> dedicated per device key as well?

So it was what I assumed. With that one static key it should be one
"lock-class" with several instances. So I don't think lockdep notices a
difference between index == 0 and index == 1.

If you have zram0 and zram1 and both have different lock chains which
could result in a deadlock from theoretical point of view but not in
reality because the zram0-swap never clash with zram1-ext4 then you
should have a dynamic key. That means add it struct zram and register
with lockdep_register_key() and assign with the lock as you did.

Sebastian