Pavel Emelianov wrote:
> Balbir Singh wrote:
>> Pavel Emelianov wrote:
>>>> +static inline bool res_counter_check_under_limit(struct res_counter *cnt)
>>>> +{
>>>> +	bool ret;
>>>> +	unsigned long flags;
>>>> +
>>>> +	spin_lock_irqsave(&cnt->lock, flags);
>>>> +	ret = res_counter_limit_check_locked(cnt);
>>> We don't have to take the lock for such a check.
>>>
>>  
>> This check without the lock could be racy and return incorrect
>> results -- leading to OOM.
> 
> Maybe. Nevertheless, if we do not trust the return value of
> try_to_free_pages() then the code should probably look like
> 
> while (1) {
> 	if (res_counter_charge() == 0)
> 		break;
> 
> 	did_progress = try_to_free_pages();
> 	if (res_counter_charge() == 0)
> 		break;
> 
> 	if (!did_progress)
> 		out_of_memory();
> }
> 

Yes, this looks better.

> But in any case we must know for sure was at least one page
> freed or not...
> 
> Thanks,
> Pavel


-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers