Re: [PATCH] mm: Introduce dedicated WQ_MEM_RECLAIM workqueue to do lru_add_drain_all

[Wang Sheng-Hui <shhuiw@foxmail.com>]

On 6/3/2016 8:48 AM, Wang Sheng-Hui wrote:
> Tejun,
>
>
> On 6/2/2016 10:39 PM, Tejun Heo wrote:
>> On Thu, Jun 02, 2016 at 03:48:51PM +0800, Wang Sheng-Hui wrote:
>>> +static int __init lru_init(void)
>>> +{
>>> +	lru_add_drain_wq = alloc_workqueue("lru-add-drain",
>>> +		WQ_MEM_RECLAIM | WQ_UNBOUND, 0);
>> Why is it unbound?
> Sorry, I just pasted from other wq create statement.
>
> WQ_MEM_RECLAIM is the key. Will drop WQ_UNBOUND in new version patch.
>
>
>>> +	if (WARN(!lru_add_drain_wq,
>>> +		"Failed to create workqueue lru_add_drain_wq"))
>>> +		return -ENOMEM;
>> I don't think we need an explicit warn here.  Doesn't error return
>> from an init function trigger boot failure anyway?
Tejun,

Seems do_initcalls =>...=> do_one_initcall will not warn on error code returned
from early_initcall functions.

Next version will reserve the warn here, but crash directly when wq was not created but used.

> Will drop the warn and return -ENOMEM directly on failure.
>>> +	return 0;
>>> +}
>>> +early_initcall(lru_init);
>>> +
>>>  void lru_add_drain_all(void)
>>>  {
>>>  	static DEFINE_MUTEX(lock);
>>>  	static struct cpumask has_work;
>>>  	int cpu;
>>>  
>>> +	struct workqueue_struct *lru_wq = lru_add_drain_wq ?: system_wq;
>>> +
>>> +	WARN_ONCE(!lru_add_drain_wq,
>>> +		"Use system_wq to do lru_add_drain_all()");
>> Ditto.  The system is crashing for sure.  What's the point of this
>> warning?
> It's for above warn failure. Will crash instead of falling back to system_wq
>
>> Thanks.
>>
> Thanks,
> Sheng-Hui



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>