On Mon, 3 Feb 2025 12:26:12 +0900 Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> wrote:
> On (25/01/31 14:55), Andrew Morton wrote:
> > > +static void zram_slot_write_lock(struct zram *zram, u32 index)
> > > +{
> > > + atomic_t *lock = &zram->table[index].lock;
> > > + int old = atomic_read(lock);
> > > +
> > > + do {
> > > + if (old != ZRAM_ENTRY_UNLOCKED) {
> > > + cond_resched();
> > > + old = atomic_read(lock);
> > > + continue;
> > > + }
> > > + } while (!atomic_try_cmpxchg(lock, &old, ZRAM_ENTRY_WRLOCKED));
> > > +}
> >
> > I expect that if the calling userspace process has realtime policy (eg
> > SCHED_FIFO) then the cond_resched() won't schedule SCHED_NORMAL tasks
> > and this becomes a busy loop. And if the machine is single-CPU, the
> > loop is infinite.
>
> So for that scenario to happen zram needs to see two writes() to the same
> index (page) simultaneously? Or read() and write() on the same index (page)
> concurrently?
Well, my point is that in the contended case, this "lock" operation can
get stuck forever. If there are no contended cases, we don't need a
lock!
And I don't see how disabling the feature if PREEMPT=y will avoid this
situation. cond_resched() won't schedule away from a realtime task to
a non-realtime one - a policy which isn't related to preemption.
