on 12/1/2022 12:54 PM, Kemeng Shi wrote: > If we decremented queue without waiters, we should not decremente freed > bits number "nr", or all "nr" could be consumed in a empty queue and no > wakeup will be called. > Currently, for case "wait_cnt > 0", "nr" will not be decremented if we > decremented queue without watiers and retry is returned to avoid lost > wakeups. However for case "wait_cnt == 0", "nr" will be decremented > unconditionally and maybe decremented to zero. Although retry is > returned by active state of queue, it's not actually executed for "nr" > is zero. > > Fix this by only decrementing "nr" for active queue when "wait_cnt == > 0". After this fix, "nr" will always be non-zero when we decremented > inactive queue for case "wait_cnt == 0", so the need to retry could > be returned by "nr" and active state of waitqueue returned for the same > purpose is not needed. > > Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxx> > --- > lib/sbitmap.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/lib/sbitmap.c b/lib/sbitmap.c > index 7280ae8ca88c..e40759bcf821 100644 > --- a/lib/sbitmap.c > +++ b/lib/sbitmap.c > @@ -604,7 +604,6 @@ static bool __sbq_wake_up(struct sbitmap_queue *sbq, int *nr) > struct sbq_wait_state *ws; > unsigned int wake_batch; > int wait_cnt, cur, sub; > - bool ret; > > if (*nr <= 0) > return false; > @@ -632,15 +631,15 @@ static bool __sbq_wake_up(struct sbitmap_queue *sbq, int *nr) > if (wait_cnt > 0) > return !waitqueue_active(&ws->wait); > > - *nr -= sub; > - > /* > * When wait_cnt == 0, we have to be particularly careful as we are > * responsible to reset wait_cnt regardless whether we've actually > - * woken up anybody. But in case we didn't wakeup anybody, we still > - * need to retry. > + * woken up anybody. But in case we didn't wakeup anybody, we should > + * not consume nr and need to retry to avoid lost wakeups. > */ > - ret = !waitqueue_active(&ws->wait); There is a warnning reported by checkpatch.pl which is "WARNING:waitqueue_active without comment" but I don't know why. > + if (waitqueue_active(&ws->wait)) > + *nr -= sub; > + > wake_batch = READ_ONCE(sbq->wake_batch); > > /* > @@ -669,7 +668,7 @@ static bool __sbq_wake_up(struct sbitmap_queue *sbq, int *nr) > sbq_index_atomic_inc(&sbq->wake_index); > atomic_set(&ws->wait_cnt, wake_batch); > > - return ret || *nr; > + return *nr; > } > > void sbitmap_queue_wake_up(struct sbitmap_queue *sbq, int nr) > Besides, there are some git config problems for my huaweicloud email, I will send patchset with huaweicloud email when I fix them. Thanks. -- Best wishes Kemeng Shi
