On Thu, Oct 19, 2023 at 07:44:32PM +0800, Hillf Danton wrote: > On Wed, 18 Oct 2023 13:20:49 +0200 Uladzislau Rezki (Sony) <[email protected]> > wrote: > > > On Mon, Oct 16, 2023 at 1:30 PM Uladzislau Rezki (Sony) > > > <[email protected]> wrote: > > > > +static void rcu_sr_normal_gp_init(void) > > > > +{ > > > > + struct llist_node *llnode, *rcu; > > > > + int ret; > > > > + > > > > + if (llist_empty(&sr.curr)) > > > > + return; > > > > > > This empty check erases the curr_tail race below instead of > > > atomic_inc_return(&sr.active), because llist_add() will never return true > > > after this check. > > > > > I use "active" counter to guarantee that a tail was updated in the > > rcu_sr_normal_add_req(), i.e. the list might be not empty whereas the > > tail updating might be in progress. llist_add() success and the task gets > > preemted as an example. > > You are right - the preempt is what I missed. > > Then another question rising - the adding order of sync requests is changed > at wakeup time, as shown by the two functions below with sr.curr_tail and > sr.active cut off. > > static void rcu_sr_normal_gp_init(void) > { > struct llist_node *llnode, *rcu; > > llnode = llist_del_all(&sr.curr); > rcu = llist_reverse_order(llnode); > if (!rcu) > return; > /* > * A waiting list of GP should be empty on this step, > * since a GP-kthread, rcu_gp_init() -> gp_cleanup(), > * rolls it over. If not, it is a BUG, warn a user. > */ > WARN_ON_ONCE(!llist_empty(&sr.wait)); > > WRITE_ONCE(sr.wait_tail, llnode); > > llist_add_batch(rcu, llnode, &sr.wait); > } > > static void rcu_sr_normal_add_req(struct rcu_synchronize *rs) > { > llist_add((struct llist_node *) &rs->head, &sr.curr); > } > This is what i was thinking of in the beginning but i had decided to make it more complex and maintain the current tail. We will get a slight penalty in performance on a synthetic test but i have compared it and this is negligible in fact.
Another good thing with it is we process users in a reverse order, i.e. the most waiting users to less ones, this is from a waiting time point of view. I like it better and it is more simple. Anyway we can improve it further. Thank you for the proposal! -- Uladzislau Rezki
