Messages in this thread | | | From | kamezawa.hiroyu@jp ... | Date | Tue, 23 Sep 2008 00:43:21 +0900 (JST) | 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
| |