On Fri 06-05-16 13:09:49, Christoph Lameter wrote:
> The cpu_stat_off variable is unecessary since we can check if
> a workqueue request is pending otherwise. Removal of
> cpu_stat_off makes it pretty easy for the vmstat shepherd to
> ensure that the proper things happen.
OK, this looks good to me. It is racy ...
vmstat_shepherd vmstat_update
delayed_work_pending() # false
need_update() refresh_cpu_vm_stats()
queue_delayed_work_on() queue_delayed_work_on()
... but it doesn't matter because queue_delayed_work_on is a noop
if the work is already queued.
> Removing the state also removes all races related to it.
Do we have any races left? I do not see any.
> Should a workqueue not be scheduled as needed for vmstat_update
> then the shepherd will notice and schedule it as needed.
> Should a workqueue be unecessarily scheduled then the vmstat
> updater will disable it.
The code simplification is really nice!
> V1->V2:
> - Rediff to proper upstream version
>
> Signed-off-by: Christoph Lameter <cl@xxxxxxxxx>
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
A nit below
> @@ -1475,20 +1454,11 @@ static void vmstat_shepherd(struct work_
>
> get_online_cpus();
> /* Check processors whose vmstat worker threads have been disabled */
> - for_each_cpu(cpu, cpu_stat_off) {
> + for_each_online_cpu(cpu) {
> struct delayed_work *dw = &per_cpu(vmstat_work, cpu);
>
> - if (need_update(cpu)) {
> - if (cpumask_test_and_clear_cpu(cpu, cpu_stat_off))
> + if (!delayed_work_pending(dw) && need_update(cpu))
> queue_delayed_work_on(cpu, vmstat_wq, dw, 0);
Indentation here...
> - } else {
> - /*
> - * Cancel the work if quiet_vmstat has put this
> - * cpu on cpu_stat_off because the work item might
> - * be still scheduled
> - */
> - cancel_delayed_work(dw);
> - }
> }
> put_online_cpus();
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>