Messages in this thread | | | Date | Fri, 19 May 2023 17:35:53 +0530 | Subject | Re: [RFC PATCH V2 1/1] sched/numa: Fix disjoint set vma scan regression | From | Raghavendra K T <> |
| |
On 5/19/2023 1:26 PM, Bharata B Rao wrote: > On 16-May-23 2:49 PM, Raghavendra K T wrote: >> With the numa scan enhancements [1], only the threads which had previously [...] >> -#define VMA_PID_RESET_PERIOD (4 * sysctl_numa_balancing_scan_delay) >> +#define VMA_PID_RESET_PERIOD (4 * sysctl_numa_balancing_scan_delay) >> +#define DISJOINT_VMA_SCAN_RENEW_THRESH 16 >> >> /* >> * The expensive part of numa migration is done from task_work context. >> @@ -3058,6 +3072,8 @@ static void task_numa_work(struct callback_head *work) >> /* Reset happens after 4 times scan delay of scan start */ >> vma->numab_state->next_pid_reset = vma->numab_state->next_scan + >> msecs_to_jiffies(VMA_PID_RESET_PERIOD); >> + >> + WRITE_ONCE(vma->numab_state->scan_counter, 0); >> } >> >> /* >> @@ -3068,6 +3084,13 @@ static void task_numa_work(struct callback_head *work) >> vma->numab_state->next_scan)) >> continue; >> >> + /* >> + * For long running tasks, renew the disjoint vma scanning >> + * periodically. >> + */ >> + if (mm->numa_scan_seq && !(mm->numa_scan_seq % DISJOINT_VMA_SCAN_RENEW_THRESH)) > > Don't you need a READ_ONCE() accessor for mm->numa_scan_seq? >
Hello Bharata,
Yes.. Thanks for pointing out.. V1 I did ensure that, But in V2 somehow leftout :( .
On the other-hand I see vma->numab_state->scan_counter does not need READ_ONCE/WRITE_ONCE since it is not modified out of this function (i.e. it is all done after cmpxchg above)..
Also thinking more, DISJOINT_VMA_SCAN_RENEW_THRESH reset change itself may need some correction, and doesn't seem to be absolutely necessary here. (will post that separately for improving long running benchmark as per my experiment with more detail)
will wait for any confirmation of reported regression fix with this patch and/or any better idea/ack for a while and repost.
| |