Re: [PATCH] mm/hugetlb: Defer freeing of huge pages if in non-task context

[Waiman Long <longman@redhat.com>]

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