linux-kernel - Re: Re: Re: Re: [PATCH 4/13] memcg: force

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <33417579.1222098201726.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Tue, 23 Sep 2008 00:43:21 +0900 (JST)
From:	kamezawa.hiroyu@...fujitsu.com
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	kamezawa.hiroyu@...fujitsu.com, linux-mm@...ck.org,
	balbir@...ux.vnet.ibm.com, nishimura@....nes.nec.co.jp,
	xemul@...nvz.org, LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: Re: Re: Re: [PATCH 4/13] memcg: force_empty moving account

----- Original Message -----
>> This force_empty is called only in following situation
>>  - there is no user threas in this cgroup.
>>  - a user tries to rmdir() this cgroup or explicitly type
>>    echo 1 > ../memory.force_empty.
>> 
>> force_empty() scans lru list of this cgroup and check page_cgroup on the
>> list one by one. Because there are no tasks in this group, force_empty can
>> see following racy condtions while scanning.
>> 
>>  - global lru tries to remove the page which pointed by page_cgroup 
>>    and it is not-on-LRU.
>
>So you either skip the page because it already got un-accounted, or you
>retry because its state is already updated to some new state.
>
>>  - the page is locked by someone.
>>    ....find some lock contetion with invalidation/truncate.
>
>Then you just contend the lock and get woken when you obtain?
>
>>  - in later patch, page_cgroup can be on pagevec(i added) and we have to dr
ain
>>    it to remove from LRU.
>
>Then unlock, drain, lock, no need to sleep some arbitrary amount of time
>[0-inf).
>
>> In above situation, force_empty() have to wait for some event proceeds.
>> 
>> Hmm...detecting busy situation in loop and sleep in out-side-of-loop
>> is better ? Anyway, ok, I'll rewrite this.
>
>The better solution is to wait for events in a non-polling fashion, for
>example by using wait_event().
>
Hmm,
spin_unlock -> wait_on_page_locked() -> break loop or spin_lock and retry
will be a candidates. I'll see how it looks.

>yield() might not actually wait at all, suppose you're the highest
>priority FIFO task on the system - if you used yield and rely on someone
>else to run you'll deadlock.
>
Oh, I missed that. ok. yield() here is bad.

>Also, depending on sysctl_sched_compat_yield, SCHED_OTHER tasks using
>yield() can behave radically different.
>
>> BTW, sched.c::yield() is for what purpose now ?
>
>There are some (lagacy) users of yield, sadly they are all incorrect,
>but removing them is non-trivial for various reasons.
>
>The -rt kernel has 2 sites where yield() is the correct thing to do. In
>both cases its where 2 SCHED_FIFO-99 tasks (migration and stop_machine)
>depend on each-other.
>

Thank you for kindly advices. I'll rewrite.

Regards,
-Kame

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/