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

[Sergey Senozhatsky]

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?