Re: [PATCH] zram: fix possible race when checking idle_strm

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

On (08/07/15 17:03), Joonsoo Kim wrote:
> Currently, when we enter the wait state due to lack of idle stream,
> we check idle_strm list without holding the lock in expanding of
> wait_event define. In this case, some one can see stale value and
> process could fall into wait state without any upcoming wakeup process.

hm... I need to think about it more.

we do wake_up every time we put stream back to the list

zcomp_strm_multi_release():

        spin_lock(&zs->strm_lock);
        if (zs->avail_strm <= zs->max_strm) {
                list_add(&zstrm->list, &zs->idle_strm);
                spin_unlock(&zs->strm_lock);
                wake_up(&zs->strm_wait);
                return;
        }


but I can probably see what you mean... in some very extreme case,
though. I can't even formulate it... eh... we use a multi stream
backend with ->max_strm == 1 and there are two processes, one
just falsely passed the wait_event() `if (condition)' check, the
other one just put stream back to ->idle_strm and called wake_up(),
but the first process hasn't yet executed prepare_to_wait_event()
so it might miss a wakeup. and there should be no other process
doing read or write operation. otherwise, there will be wakeup
eventually.

is this the case you were thinking of?... then yes, this spinlock
may help.

hm... we also can simply forbid downgrading a multi stream backend
to a single stream (in terms of performance it is much slower
than a real single steam anyway). will this do the trick? if we
have more than 1 stream and idle list then there will be more that
one wakeup. and every woken up task will call wakeup once it put
stream.

I don't think I'll be able to check the email until this Sunday,
so here is my

Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

to this patch if Minchan decides that `forbid downgrading` is
hacky or doesn't work.

very nice finding, Joonsoo!


p.s. hm... we can take a look at 'forbid downgrading a multi
stream backend to a single-streamed'.

	-ss