Balbir Singh
2016-Apr-04 05:53 UTC
[PATCH v3 01/16] mm: use put_page to free page instead of putback_lru_page
On 30/03/16 18:12, Minchan Kim wrote:> Procedure of page migration is as follows: > > First of all, it should isolate a page from LRU and try to > migrate the page. If it is successful, it releases the page > for freeing. Otherwise, it should put the page back to LRU > list. > > For LRU pages, we have used putback_lru_page for both freeing > and putback to LRU list. It's okay because put_page is aware of > LRU list so if it releases last refcount of the page, it removes > the page from LRU list. However, It makes unnecessary operations > (e.g., lru_cache_add, pagevec and flags operations. It would be > not significant but no worth to do) and harder to support new > non-lru page migration because put_page isn't aware of non-lru > page's data structure. > > To solve the problem, we can add new hook in put_page with > PageMovable flags check but it can increase overhead in > hot path and needs new locking scheme to stabilize the flag check > with put_page. > > So, this patch cleans it up to divide two semantic(ie, put and putback). > If migration is successful, use put_page instead of putback_lru_page and > use putback_lru_page only on failure. That makes code more readable > and doesn't add overhead in put_page.So effectively when we return from unmap_and_move() the page is either put_page or putback_lru_page() and the page is gone from under us.
Minchan Kim
2016-Apr-04 06:01 UTC
[PATCH v3 01/16] mm: use put_page to free page instead of putback_lru_page
On Mon, Apr 04, 2016 at 03:53:59PM +1000, Balbir Singh wrote:> > > On 30/03/16 18:12, Minchan Kim wrote: > > Procedure of page migration is as follows: > > > > First of all, it should isolate a page from LRU and try to > > migrate the page. If it is successful, it releases the page > > for freeing. Otherwise, it should put the page back to LRU > > list. > > > > For LRU pages, we have used putback_lru_page for both freeing > > and putback to LRU list. It's okay because put_page is aware of > > LRU list so if it releases last refcount of the page, it removes > > the page from LRU list. However, It makes unnecessary operations > > (e.g., lru_cache_add, pagevec and flags operations. It would be > > not significant but no worth to do) and harder to support new > > non-lru page migration because put_page isn't aware of non-lru > > page's data structure. > > > > To solve the problem, we can add new hook in put_page with > > PageMovable flags check but it can increase overhead in > > hot path and needs new locking scheme to stabilize the flag check > > with put_page. > > > > So, this patch cleans it up to divide two semantic(ie, put and putback). > > If migration is successful, use put_page instead of putback_lru_page and > > use putback_lru_page only on failure. That makes code more readable > > and doesn't add overhead in put_page. > So effectively when we return from unmap_and_move() the page is either > put_page or putback_lru_page() and the page is gone from under us.I didn't get your point. Could you elaborate it more what you want to say about this patch?
Balbir Singh
2016-Apr-05 03:10 UTC
[PATCH v3 01/16] mm: use put_page to free page instead of putback_lru_page
On 04/04/16 16:01, Minchan Kim wrote:> On Mon, Apr 04, 2016 at 03:53:59PM +1000, Balbir Singh wrote: >> >> On 30/03/16 18:12, Minchan Kim wrote: >>> Procedure of page migration is as follows: >>> >>> First of all, it should isolate a page from LRU and try to >>> migrate the page. If it is successful, it releases the page >>> for freeing. Otherwise, it should put the page back to LRU >>> list. >>> >>> For LRU pages, we have used putback_lru_page for both freeing >>> and putback to LRU list. It's okay because put_page is aware of >>> LRU list so if it releases last refcount of the page, it removes >>> the page from LRU list. However, It makes unnecessary operations >>> (e.g., lru_cache_add, pagevec and flags operations. It would be >>> not significant but no worth to do) and harder to support new >>> non-lru page migration because put_page isn't aware of non-lru >>> page's data structure. >>> >>> To solve the problem, we can add new hook in put_page with >>> PageMovable flags check but it can increase overhead in >>> hot path and needs new locking scheme to stabilize the flag check >>> with put_page. >>> >>> So, this patch cleans it up to divide two semantic(ie, put and putback). >>> If migration is successful, use put_page instead of putback_lru_page and >>> use putback_lru_page only on failure. That makes code more readable >>> and doesn't add overhead in put_page. >> So effectively when we return from unmap_and_move() the page is either >> put_page or putback_lru_page() and the page is gone from under us. > I didn't get your point. > Could you elaborate it more what you want to say about this patch?I was just adding to my understanding of this change based on your changelog. My understanding is that we take the extra reference in isolate_lru_page() but by the time we return from unmap_and_move() we drop the extra reference Balbir Singh
Seemingly Similar Threads
- [PATCH v3 01/16] mm: use put_page to free page instead of putback_lru_page
- [PATCH v3 01/16] mm: use put_page to free page instead of putback_lru_page
- [PATCH v1 01/19] mm: use put_page to free page instead of putback_lru_page
- [PATCH v3 01/16] mm: use put_page to free page instead of putback_lru_page
- [PATCH v3 01/16] mm: use put_page to free page instead of putback_lru_page