On 12/16/19 6:20 PM, Waiman Long wrote: >>> + >>> + /* >>> + * Racing may prevent some deferred huge pages in hpage_freelist >>> + * from being freed. Check here and call schedule_work() if that >>> + * is the case. >>> + */ >>> + if (unlikely(hpage_freelist && !work_pending(&free_hpage_work))) >>> + schedule_work(&free_hpage_work); >> Can you describe the race which would leave deferred huge pages on >> hpage_freelist? I am having a hard time determining how that can happen. > I am being cautious here. It is related how the workqueue works. Whether > a call to schedule_work() has any effect depends on the pending bit in > the workqueue structure. I suppose that it is cleared once the work is > done. So depending on when the bit is cleared, there may be a small > timing window where free_hpage_workfn() is done but the bit has not been > cleared yet. A concurrent softIRQ task may update hpage_freelist and > call schedule_work() without actually queuing it. Perhaps I can check > the return status of schedule_work() and wait for a while there until > the queuing is successfully or the free list is changed. I will need to > look more carefully at the workqueue code to see how big this timing > window is. >> And, if this indeed can happen then I would have to ask what happens if >> a page is 'stuck' and we do not call free_huge_page? Do we need to take >> that case into account? > As said above, there may be way to reduce the racing window or eliminate > it altogether. I need a bit more time to investigate that. If there is > no way to eliminate the racing window, it is possible that a huge page > may get stuck in the free list for a while. My mistake. The pending bit is actually cleared before calling the workfn. That shows I don't fully understand the work queue functionality. In this case, there should be no race. I will remove the unnecessary check. Cheers, Longman
