On (25/02/13 19:32), Hillf Danton wrote:
[..]
> > +static void zspage_read_lock(struct zspage *zspage)
> > +{
> > + atomic_t *lock = &zspage->lock;
> > + int old = atomic_read_acquire(lock);
> > +
> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > + rwsem_acquire_read(&zspage->lockdep_map, 0, 0, _RET_IP_);
> > +#endif
> > +
> > + do {
> > + if (old == ZS_PAGE_WRLOCKED) {
> > + cpu_relax();
> > + old = atomic_read_acquire(lock);
> > + continue;
> > + }
> > + } while (!atomic_try_cmpxchg_acquire(lock, &old, old + 1));
>
> Given mcs_spinlock, inventing spinlock in 2025 sounds no good.
> See below for the spinlock version.
I should have sent this series in 2024, when inventing a spinlock
sounded good :)
> struct zspage_lock {
> spinlock_t lock;
> int cnt;
> struct lockdep_map lockdep_map;
> };
>
> static __must_check bool zspage_write_trylock(struct zspage_lock *zl)
> {
> spin_lock(&zl->lock);
> if (zl->cnt == ZS_PAGE_UNLOCKED) {
> // zl->cnt = ZS_PAGE_WRLOCKED;
> rwsem_acquire(&zl->lockdep_map, 0, 1, _RET_IP_);
> return true;
> }
> spin_unlock(&zl->lock);
> return false;
> }
>
> static void zspage_write_unlock(struct zspage_lock *zl)
> {
> rwsem_release(&zl->lockdep_map, _RET_IP_);
> spin_unlock(&zl->lock);
> }
>
> static void zspage_read_lock(struct zspage_lock *zl)
> {
> rwsem_acquire_read(&zl->lockdep_map, 0, 0, _RET_IP_);
>
> spin_lock(&zl->lock);
> zl->cnt++;
> spin_unlock(&zl->lock);
> }
>
> static void zspage_read_unlock(struct zspage_lock *zl)
> {
> rwsem_release(&zl->lockdep_map, _RET_IP_);
>
> spin_lock(&zl->lock);
> zl->cnt--;
> spin_unlock(&zl->lock);
> }
I see, yeah I can pick it up, thanks. A couple of *minor* things I can
think of. First. in the current implementation I also track LOCK_STAT
(lock-contended/lock-acquired), something like
static inline void __read_lock(struct zspage *zspage)
{
atomic_t *lock = &zspage->lock;
int old = atomic_read_acquire(lock);
rwsem_acquire_read(&zspage->dep_map, 0, 0, _RET_IP_);
do {
if (old == ZS_PAGE_WRLOCKED) {
lock_contended(&zspage->dep_map, _RET_IP_);
cpu_relax();
old = atomic_read_acquire(lock);
continue;
}
} while (!atomic_try_cmpxchg_acquire(lock, &old, old + 1));
lock_acquired(&zspage->dep_map, _RET_IP_);
}
I'll add lock-stat to zsl, but it's worth mentioning that zsl "splits"
the stats into zsl spin-lock's dep_map and zsl's own dep_map:
class name con-bounces contentions waittime-min waittime-max waittime-total waittime-avg acq-bounces acquisitions holdtime-min holdtime-max holdtime-total holdtime-avg
zspage->lock-R: 0 0 0.00 0.00 0.00 0.00 1 2 6.19 11.61 17.80 8.90
&zspage->zsl.lock: 0 0 0.00 0.00 0.00 0.00 5457 1330106 0.10 118.53 174917.46 0.13
That is, quite likely, fine. One can just add the numbers, I assume.
Second, we'll be carrying around two dep_map-s per-zsl in lockdep builds
now, but, again, that is, likely, not a problem as sizeof(lockdep_map)
isn't too huge (around 48 bytes).
