>
> A couple more questions interspersed below upon further reflection.
>
> Thoughts?
>
See below my thoughts:
> Thanx, Paul
>
> > ---
> > kernel/rcu/tree.c | 47 +++++++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 39 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index c94c17194299..44279ca488ef 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2741,11 +2741,13 @@ EXPORT_SYMBOL_GPL(call_rcu);
> > /**
> > * struct kvfree_rcu_bulk_data - single block to store kvfree_rcu() pointers
> > * @list: List node. All blocks are linked between each other
> > + * @gp_snap: Snapshot of RCU state for objects placed to this bulk
> > * @nr_records: Number of active pointers in the array
> > * @records: Array of the kvfree_rcu() pointers
> > */
> > struct kvfree_rcu_bulk_data {
> > struct list_head list;
> > + unsigned long gp_snap;
> > unsigned long nr_records;
> > void *records[];
> > };
> > @@ -2762,13 +2764,15 @@ struct kvfree_rcu_bulk_data {
> > * struct kfree_rcu_cpu_work - single batch of kfree_rcu() requests
> > * @rcu_work: Let queue_rcu_work() invoke workqueue handler after grace period
> > * @head_free: List of kfree_rcu() objects waiting for a grace period
> > + * @head_free_gp_snap: Snapshot of RCU state for objects placed to "@head_free"
> > * @bulk_head_free: Bulk-List of kvfree_rcu() objects waiting for a grace period
> > * @krcp: Pointer to @kfree_rcu_cpu structure
> > */
> >
> > struct kfree_rcu_cpu_work {
> > - struct rcu_work rcu_work;
> > + struct work_struct rcu_work;
> > struct rcu_head *head_free;
> > + unsigned long head_free_gp_snap;
> > struct list_head bulk_head_free[FREE_N_CHANNELS];
> > struct kfree_rcu_cpu *krcp;
> > };
> > @@ -2964,10 +2968,11 @@ static void kfree_rcu_work(struct work_struct *work)
> > struct rcu_head *head;
> > struct kfree_rcu_cpu *krcp;
> > struct kfree_rcu_cpu_work *krwp;
> > + unsigned long head_free_gp_snap;
> > int i;
> >
> > - krwp = container_of(to_rcu_work(work),
> > - struct kfree_rcu_cpu_work, rcu_work);
> > + krwp = container_of(work,
> > + struct kfree_rcu_cpu_work, rcu_work);
> > krcp = krwp->krcp;
> >
> > raw_spin_lock_irqsave(&krcp->lock, flags);
> > @@ -2978,12 +2983,29 @@ static void kfree_rcu_work(struct work_struct *work)
> > // Channel 3.
> > head = krwp->head_free;
> > krwp->head_free = NULL;
> > + head_free_gp_snap = krwp->head_free_gp_snap;
> > raw_spin_unlock_irqrestore(&krcp->lock, flags);
> >
> > // Handle the first two channels.
> > - for (i = 0; i < FREE_N_CHANNELS; i++)
> > + for (i = 0; i < FREE_N_CHANNELS; i++) {
> > + // Start from the tail page, so a GP is likely passed for it.
> > + list_for_each_entry_safe_reverse(bnode, n, &bulk_head[i], list) {
> > + // Not yet ready? Bail out since we need one more GP.
> > + if (!poll_state_synchronize_rcu(bnode->gp_snap))
> > + break;
> > +
> > + list_del_init(&bnode->list);
> > + kvfree_rcu_bulk(krcp, bnode, i);
> > + }
> > +
> > + // Please note a request for one more extra GP can
> > + // occur only once for all objects in this batch.
> > + if (!list_empty(&bulk_head[i]))
> > + synchronize_rcu();
>
> Does directly invoking synchronize_rcu() instead of using queue_rcu_work()
> provide benefits, for example, reduced memory footprint?
>
queue_rcu_work() will delay freeing of all objects in a batch. We can
make use of it but it should be only for the ones which still require
a grace period. A memory footprint and a time depends on when our
callback is invoked by the RCU-core to queue the reclaim work.
Such time can be long, because it depends on many factors:
- scheduling delays in waking gp;
- scheduling delays in kicking nocb;
- delays in waiting in a "cblist":
- dequeuing and invoking f(rhp);
- delay in waking our final reclaim work and giving it a CPU time.
This patch combines a possibility to reclaim asap for objects which
passed a grace period and requesting one more GP for the ones which
have not passed it yet.
>
> If not, it would be good to instead use queue_rcu_work() in order
> to avoid an unnecessary context switch in this workqueue handler.
>
I went by the most easiest way from code perspective since i do not
see problems with a current approach from testing and personal point
of views.
If we are about to do that i need to add extra logic to split ready
and not ready pointers for direct reclaim and the rest over the
queu_rcu_work().
I can check how it goes.
>
> My concern is that an RCU CPU stall might otherwise end up tying up more
> workqueue kthreads as well as more memory.
>
There is a limit. We have two batches, one work for each. Suppose the
reclaim kthread is stuck in synchronize_rcu() so it does not do any
progress. In this case same work can be only in pending state and
nothing more no matter how many times the queue_work() is invoked:
2 * num_possible_cpus();
If we end up in RCU stall we will not be able to reclaim anyway.
--
Uladzislau Rezki
