* isolate_lru_page on !head pages @ 2015-12-09 13:02 ` Michal Hocko 0 siblings, 0 replies; 12+ messages in thread From: Michal Hocko @ 2015-12-09 13:02 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML Hi Kirill, while looking at the issue reported by Minchan [1] I have noticed that there is nothing to prevent from "isolating" a tail page from LRU because isolate_lru_page checks PageLRU which is PAGEFLAG(LRU, lru, PF_HEAD) so it is checked on the head page rather than the given page directly but the rest of the operation is done on the given (tail) page. This is really subtle because this expects that every caller of this function checks for the tail page otherwise we would clobber statistics and who knows what else (I haven't checked that in detail) as the page cannot be on the LRU list and the operation makes sense only on the head page. Would it make more sense to make PageLRU PF_ANY? That would return false for PageLRU on any tail page and so it would be ignored by isolate_lru_page. I haven't checked other flags but there might be a similar situation. I am wondering whether it is really a good idea to perform a flag check on a different page then the operation which depends on the result of the test in general. It sounds like a maintenance horror to me. [1] http://lkml.kernel.org/r/20151201133455.GB27574@bbox -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 12+ messages in thread
* isolate_lru_page on !head pages @ 2015-12-09 13:02 ` Michal Hocko 0 siblings, 0 replies; 12+ messages in thread From: Michal Hocko @ 2015-12-09 13:02 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML Hi Kirill, while looking at the issue reported by Minchan [1] I have noticed that there is nothing to prevent from "isolating" a tail page from LRU because isolate_lru_page checks PageLRU which is PAGEFLAG(LRU, lru, PF_HEAD) so it is checked on the head page rather than the given page directly but the rest of the operation is done on the given (tail) page. This is really subtle because this expects that every caller of this function checks for the tail page otherwise we would clobber statistics and who knows what else (I haven't checked that in detail) as the page cannot be on the LRU list and the operation makes sense only on the head page. Would it make more sense to make PageLRU PF_ANY? That would return false for PageLRU on any tail page and so it would be ignored by isolate_lru_page. I haven't checked other flags but there might be a similar situation. I am wondering whether it is really a good idea to perform a flag check on a different page then the operation which depends on the result of the test in general. It sounds like a maintenance horror to me. [1] http://lkml.kernel.org/r/20151201133455.GB27574@bbox -- Michal Hocko SUSE Labs -- 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages 2015-12-09 13:02 ` Michal Hocko @ 2015-12-14 12:04 ` Kirill A. Shutemov -1 siblings, 0 replies; 12+ messages in thread From: Kirill A. Shutemov @ 2015-12-14 12:04 UTC (permalink / raw) To: Michal Hocko; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > Hi Kirill, [ sorry for late reply, just back from vacation. ] > while looking at the issue reported by Minchan [1] I have noticed that > there is nothing to prevent from "isolating" a tail page from LRU because > isolate_lru_page checks PageLRU which is > PAGEFLAG(LRU, lru, PF_HEAD) > so it is checked on the head page rather than the given page directly > but the rest of the operation is done on the given (tail) page. Looks like most (all?) callers already exclude PTE-mapped THP already one way or another. Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would be appropriate. > This is really subtle because this expects that every caller of this > function checks for the tail page otherwise we would clobber statistics > and who knows what else (I haven't checked that in detail) as the page > cannot be on the LRU list and the operation makes sense only on the head > page. > > Would it make more sense to make PageLRU PF_ANY? That would return > false for PageLRU on any tail page and so it would be ignored by > isolate_lru_page. I don't think this is right way to go. What we put on LRU is compound page, not 4k subpages. PageLRU() should return true if the compound page is on LRU regardless if you ask for head or tail page. False-negatives PageLRU() can be as bad as bug Minchan reported, but perhaps more silent. > I haven't checked other flags but there might be a similar situation. I > am wondering whether it is really a good idea to perform a flag check on > a different page then the operation which depends on the result of the > test in general. It sounds like a maintenance horror to me. > > [1] http://lkml.kernel.org/r/20151201133455.GB27574@bbox -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages @ 2015-12-14 12:04 ` Kirill A. Shutemov 0 siblings, 0 replies; 12+ messages in thread From: Kirill A. Shutemov @ 2015-12-14 12:04 UTC (permalink / raw) To: Michal Hocko; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > Hi Kirill, [ sorry for late reply, just back from vacation. ] > while looking at the issue reported by Minchan [1] I have noticed that > there is nothing to prevent from "isolating" a tail page from LRU because > isolate_lru_page checks PageLRU which is > PAGEFLAG(LRU, lru, PF_HEAD) > so it is checked on the head page rather than the given page directly > but the rest of the operation is done on the given (tail) page. Looks like most (all?) callers already exclude PTE-mapped THP already one way or another. Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would be appropriate. > This is really subtle because this expects that every caller of this > function checks for the tail page otherwise we would clobber statistics > and who knows what else (I haven't checked that in detail) as the page > cannot be on the LRU list and the operation makes sense only on the head > page. > > Would it make more sense to make PageLRU PF_ANY? That would return > false for PageLRU on any tail page and so it would be ignored by > isolate_lru_page. I don't think this is right way to go. What we put on LRU is compound page, not 4k subpages. PageLRU() should return true if the compound page is on LRU regardless if you ask for head or tail page. False-negatives PageLRU() can be as bad as bug Minchan reported, but perhaps more silent. > I haven't checked other flags but there might be a similar situation. I > am wondering whether it is really a good idea to perform a flag check on > a different page then the operation which depends on the result of the > test in general. It sounds like a maintenance horror to me. > > [1] http://lkml.kernel.org/r/20151201133455.GB27574@bbox -- Kirill A. Shutemov -- 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages 2015-12-14 12:04 ` Kirill A. Shutemov @ 2015-12-15 8:52 ` Michal Hocko -1 siblings, 0 replies; 12+ messages in thread From: Michal Hocko @ 2015-12-15 8:52 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Mon 14-12-15 14:04:56, Kirill A. Shutemov wrote: > On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > > Hi Kirill, > > [ sorry for late reply, just back from vacation. ] > > > while looking at the issue reported by Minchan [1] I have noticed that > > there is nothing to prevent from "isolating" a tail page from LRU because > > isolate_lru_page checks PageLRU which is > > PAGEFLAG(LRU, lru, PF_HEAD) > > so it is checked on the head page rather than the given page directly > > but the rest of the operation is done on the given (tail) page. > > Looks like most (all?) callers already exclude PTE-mapped THP already one > way or another. I can see e.g. do_move_page_to_node_array not doing a similar thing. It isolates and then migrates potentially a tail page. I haven't looked closer whether there is other hand break on the way though. The point I was trying to make is that this is really _subtle_. We are changing something else than we operate later on. > Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would > be appropriate. > > > This is really subtle because this expects that every caller of this > > function checks for the tail page otherwise we would clobber statistics > > and who knows what else (I haven't checked that in detail) as the page > > cannot be on the LRU list and the operation makes sense only on the head > > page. > > > > Would it make more sense to make PageLRU PF_ANY? That would return > > false for PageLRU on any tail page and so it would be ignored by > > isolate_lru_page. > > I don't think this is right way to go. What we put on LRU is compound > page, not 4k subpages. PageLRU() should return true if the compound page > is on LRU regardless if you ask for head or tail page. Hmm, but then we should operate on the head page because that is what PageLRU operated on, no? > False-negatives PageLRU() can be as bad as bug Minchan reported, but > perhaps more silent. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages @ 2015-12-15 8:52 ` Michal Hocko 0 siblings, 0 replies; 12+ messages in thread From: Michal Hocko @ 2015-12-15 8:52 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Mon 14-12-15 14:04:56, Kirill A. Shutemov wrote: > On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > > Hi Kirill, > > [ sorry for late reply, just back from vacation. ] > > > while looking at the issue reported by Minchan [1] I have noticed that > > there is nothing to prevent from "isolating" a tail page from LRU because > > isolate_lru_page checks PageLRU which is > > PAGEFLAG(LRU, lru, PF_HEAD) > > so it is checked on the head page rather than the given page directly > > but the rest of the operation is done on the given (tail) page. > > Looks like most (all?) callers already exclude PTE-mapped THP already one > way or another. I can see e.g. do_move_page_to_node_array not doing a similar thing. It isolates and then migrates potentially a tail page. I haven't looked closer whether there is other hand break on the way though. The point I was trying to make is that this is really _subtle_. We are changing something else than we operate later on. > Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would > be appropriate. > > > This is really subtle because this expects that every caller of this > > function checks for the tail page otherwise we would clobber statistics > > and who knows what else (I haven't checked that in detail) as the page > > cannot be on the LRU list and the operation makes sense only on the head > > page. > > > > Would it make more sense to make PageLRU PF_ANY? That would return > > false for PageLRU on any tail page and so it would be ignored by > > isolate_lru_page. > > I don't think this is right way to go. What we put on LRU is compound > page, not 4k subpages. PageLRU() should return true if the compound page > is on LRU regardless if you ask for head or tail page. Hmm, but then we should operate on the head page because that is what PageLRU operated on, no? > False-negatives PageLRU() can be as bad as bug Minchan reported, but > perhaps more silent. -- Michal Hocko SUSE Labs -- 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages 2015-12-15 8:52 ` Michal Hocko @ 2015-12-15 12:03 ` Kirill A. Shutemov -1 siblings, 0 replies; 12+ messages in thread From: Kirill A. Shutemov @ 2015-12-15 12:03 UTC (permalink / raw) To: Michal Hocko; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Tue, Dec 15, 2015 at 09:52:33AM +0100, Michal Hocko wrote: > On Mon 14-12-15 14:04:56, Kirill A. Shutemov wrote: > > On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > > > Hi Kirill, > > > > [ sorry for late reply, just back from vacation. ] > > > > > while looking at the issue reported by Minchan [1] I have noticed that > > > there is nothing to prevent from "isolating" a tail page from LRU because > > > isolate_lru_page checks PageLRU which is > > > PAGEFLAG(LRU, lru, PF_HEAD) > > > so it is checked on the head page rather than the given page directly > > > but the rest of the operation is done on the given (tail) page. > > > > Looks like most (all?) callers already exclude PTE-mapped THP already one > > way or another. > > I can see e.g. do_move_page_to_node_array not doing a similar thing. It > isolates and then migrates potentially a tail page. No, it doesn't. follow_page(FOLL_SPLIT) would split THP pages. > I haven't looked closer whether there is other hand break on the way > though. The point I was trying to make is that this is really _subtle_. > We are changing something else than we operate later on. > > > Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would > > be appropriate. > > > > > This is really subtle because this expects that every caller of this > > > function checks for the tail page otherwise we would clobber statistics > > > and who knows what else (I haven't checked that in detail) as the page > > > cannot be on the LRU list and the operation makes sense only on the head > > > page. > > > > > > Would it make more sense to make PageLRU PF_ANY? That would return > > > false for PageLRU on any tail page and so it would be ignored by > > > isolate_lru_page. > > > > I don't think this is right way to go. What we put on LRU is compound > > page, not 4k subpages. PageLRU() should return true if the compound page > > is on LRU regardless if you ask for head or tail page. > > Hmm, but then we should operate on the head page because that is what > PageLRU operated on, no? head page is what linked into LRU, but not nessesary the way we obtain the page to check. If we check PageLRU(pte_page(*pte)) it should produce the right result. > > False-negatives PageLRU() can be as bad as bug Minchan reported, but > > perhaps more silent. > > -- > Michal Hocko > SUSE Labs -- Kirill A. Shutemov ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages @ 2015-12-15 12:03 ` Kirill A. Shutemov 0 siblings, 0 replies; 12+ messages in thread From: Kirill A. Shutemov @ 2015-12-15 12:03 UTC (permalink / raw) To: Michal Hocko; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Tue, Dec 15, 2015 at 09:52:33AM +0100, Michal Hocko wrote: > On Mon 14-12-15 14:04:56, Kirill A. Shutemov wrote: > > On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > > > Hi Kirill, > > > > [ sorry for late reply, just back from vacation. ] > > > > > while looking at the issue reported by Minchan [1] I have noticed that > > > there is nothing to prevent from "isolating" a tail page from LRU because > > > isolate_lru_page checks PageLRU which is > > > PAGEFLAG(LRU, lru, PF_HEAD) > > > so it is checked on the head page rather than the given page directly > > > but the rest of the operation is done on the given (tail) page. > > > > Looks like most (all?) callers already exclude PTE-mapped THP already one > > way or another. > > I can see e.g. do_move_page_to_node_array not doing a similar thing. It > isolates and then migrates potentially a tail page. No, it doesn't. follow_page(FOLL_SPLIT) would split THP pages. > I haven't looked closer whether there is other hand break on the way > though. The point I was trying to make is that this is really _subtle_. > We are changing something else than we operate later on. > > > Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would > > be appropriate. > > > > > This is really subtle because this expects that every caller of this > > > function checks for the tail page otherwise we would clobber statistics > > > and who knows what else (I haven't checked that in detail) as the page > > > cannot be on the LRU list and the operation makes sense only on the head > > > page. > > > > > > Would it make more sense to make PageLRU PF_ANY? That would return > > > false for PageLRU on any tail page and so it would be ignored by > > > isolate_lru_page. > > > > I don't think this is right way to go. What we put on LRU is compound > > page, not 4k subpages. PageLRU() should return true if the compound page > > is on LRU regardless if you ask for head or tail page. > > Hmm, but then we should operate on the head page because that is what > PageLRU operated on, no? head page is what linked into LRU, but not nessesary the way we obtain the page to check. If we check PageLRU(pte_page(*pte)) it should produce the right result. > > False-negatives PageLRU() can be as bad as bug Minchan reported, but > > perhaps more silent. > > -- > Michal Hocko > SUSE Labs -- Kirill A. Shutemov -- 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages 2015-12-15 12:03 ` Kirill A. Shutemov @ 2015-12-15 16:59 ` Michal Hocko -1 siblings, 0 replies; 12+ messages in thread From: Michal Hocko @ 2015-12-15 16:59 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Tue 15-12-15 14:03:18, Kirill A. Shutemov wrote: > On Tue, Dec 15, 2015 at 09:52:33AM +0100, Michal Hocko wrote: > > On Mon 14-12-15 14:04:56, Kirill A. Shutemov wrote: > > > On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > > > > Hi Kirill, > > > > > > [ sorry for late reply, just back from vacation. ] > > > > > > > while looking at the issue reported by Minchan [1] I have noticed that > > > > there is nothing to prevent from "isolating" a tail page from LRU because > > > > isolate_lru_page checks PageLRU which is > > > > PAGEFLAG(LRU, lru, PF_HEAD) > > > > so it is checked on the head page rather than the given page directly > > > > but the rest of the operation is done on the given (tail) page. > > > > > > Looks like most (all?) callers already exclude PTE-mapped THP already one > > > way or another. > > > > I can see e.g. do_move_page_to_node_array not doing a similar thing. It > > isolates and then migrates potentially a tail page. > > No, it doesn't. follow_page(FOLL_SPLIT) would split THP pages. Ahh, I thought it would split the pmd but this path splits the page directly. > > I haven't looked closer whether there is other hand break on the way > > though. The point I was trying to make is that this is really _subtle_. > > We are changing something else than we operate later on. > > > > > Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would > > > be appropriate. > > > > > > > This is really subtle because this expects that every caller of this > > > > function checks for the tail page otherwise we would clobber statistics > > > > and who knows what else (I haven't checked that in detail) as the page > > > > cannot be on the LRU list and the operation makes sense only on the head > > > > page. > > > > > > > > Would it make more sense to make PageLRU PF_ANY? That would return > > > > false for PageLRU on any tail page and so it would be ignored by > > > > isolate_lru_page. > > > > > > I don't think this is right way to go. What we put on LRU is compound > > > page, not 4k subpages. PageLRU() should return true if the compound page > > > is on LRU regardless if you ask for head or tail page. > > > > Hmm, but then we should operate on the head page because that is what > > PageLRU operated on, no? > > head page is what linked into LRU, but not nessesary the way we obtain the > page to check. If we check PageLRU(pte_page(*pte)) it should produce the > right result. I am not following you here. Any pfn walk could get to a tail page and if we happen to do e.g. isolate_lru_page we have to remember that we should always treat compound page differently. This is subtle. Anyway I am far from understading other parts of the refcount rework so I will spend time studying the code as soon as the time permits. In the meantime I agree that VM_BUG_ON_PAGE(PageTail(page), page) would be useful to catch all the fallouts. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages @ 2015-12-15 16:59 ` Michal Hocko 0 siblings, 0 replies; 12+ messages in thread From: Michal Hocko @ 2015-12-15 16:59 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On Tue 15-12-15 14:03:18, Kirill A. Shutemov wrote: > On Tue, Dec 15, 2015 at 09:52:33AM +0100, Michal Hocko wrote: > > On Mon 14-12-15 14:04:56, Kirill A. Shutemov wrote: > > > On Wed, Dec 09, 2015 at 02:02:05PM +0100, Michal Hocko wrote: > > > > Hi Kirill, > > > > > > [ sorry for late reply, just back from vacation. ] > > > > > > > while looking at the issue reported by Minchan [1] I have noticed that > > > > there is nothing to prevent from "isolating" a tail page from LRU because > > > > isolate_lru_page checks PageLRU which is > > > > PAGEFLAG(LRU, lru, PF_HEAD) > > > > so it is checked on the head page rather than the given page directly > > > > but the rest of the operation is done on the given (tail) page. > > > > > > Looks like most (all?) callers already exclude PTE-mapped THP already one > > > way or another. > > > > I can see e.g. do_move_page_to_node_array not doing a similar thing. It > > isolates and then migrates potentially a tail page. > > No, it doesn't. follow_page(FOLL_SPLIT) would split THP pages. Ahh, I thought it would split the pmd but this path splits the page directly. > > I haven't looked closer whether there is other hand break on the way > > though. The point I was trying to make is that this is really _subtle_. > > We are changing something else than we operate later on. > > > > > Probably, VM_BUG_ON_PAGE(PageTail(page), page) in isolate_lru_page() would > > > be appropriate. > > > > > > > This is really subtle because this expects that every caller of this > > > > function checks for the tail page otherwise we would clobber statistics > > > > and who knows what else (I haven't checked that in detail) as the page > > > > cannot be on the LRU list and the operation makes sense only on the head > > > > page. > > > > > > > > Would it make more sense to make PageLRU PF_ANY? That would return > > > > false for PageLRU on any tail page and so it would be ignored by > > > > isolate_lru_page. > > > > > > I don't think this is right way to go. What we put on LRU is compound > > > page, not 4k subpages. PageLRU() should return true if the compound page > > > is on LRU regardless if you ask for head or tail page. > > > > Hmm, but then we should operate on the head page because that is what > > PageLRU operated on, no? > > head page is what linked into LRU, but not nessesary the way we obtain the > page to check. If we check PageLRU(pte_page(*pte)) it should produce the > right result. I am not following you here. Any pfn walk could get to a tail page and if we happen to do e.g. isolate_lru_page we have to remember that we should always treat compound page differently. This is subtle. Anyway I am far from understading other parts of the refcount rework so I will spend time studying the code as soon as the time permits. In the meantime I agree that VM_BUG_ON_PAGE(PageTail(page), page) would be useful to catch all the fallouts. -- Michal Hocko SUSE Labs -- 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages 2015-12-15 16:59 ` Michal Hocko @ 2015-12-22 15:47 ` Vlastimil Babka -1 siblings, 0 replies; 12+ messages in thread From: Vlastimil Babka @ 2015-12-22 15:47 UTC (permalink / raw) To: Michal Hocko, Kirill A. Shutemov Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On 12/15/2015 05:59 PM, Michal Hocko wrote: >> >> head page is what linked into LRU, but not nessesary the way we obtain the >> page to check. If we check PageLRU(pte_page(*pte)) it should produce the >> right result. > > I am not following you here. Any pfn walk could get to a tail page and > if we happen to do e.g. isolate_lru_page we have to remember that we > should always treat compound page differently. This is subtle. I think the problem is that isolate_lru_page() is not the only reason for calling PageLRU(). And the other use cases have different expectations, to either way (PF_ANY or PF_HEAD) you pick for PageLRU(), somebody will have to be careful. IMHO usually it's pfn scanners who have to be careful for many reasons... > Anyway I > am far from understading other parts of the refcount rework so I will > spend time studying the code as soon as the time permits. In the > meantime I agree that VM_BUG_ON_PAGE(PageTail(page), page) would be > useful to catch all the fallouts. +1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isolate_lru_page on !head pages @ 2015-12-22 15:47 ` Vlastimil Babka 0 siblings, 0 replies; 12+ messages in thread From: Vlastimil Babka @ 2015-12-22 15:47 UTC (permalink / raw) To: Michal Hocko, Kirill A. Shutemov Cc: Andrew Morton, Minchan Kim, linux-mm, LKML On 12/15/2015 05:59 PM, Michal Hocko wrote: >> >> head page is what linked into LRU, but not nessesary the way we obtain the >> page to check. If we check PageLRU(pte_page(*pte)) it should produce the >> right result. > > I am not following you here. Any pfn walk could get to a tail page and > if we happen to do e.g. isolate_lru_page we have to remember that we > should always treat compound page differently. This is subtle. I think the problem is that isolate_lru_page() is not the only reason for calling PageLRU(). And the other use cases have different expectations, to either way (PF_ANY or PF_HEAD) you pick for PageLRU(), somebody will have to be careful. IMHO usually it's pfn scanners who have to be careful for many reasons... > Anyway I > am far from understading other parts of the refcount rework so I will > spend time studying the code as soon as the time permits. In the > meantime I agree that VM_BUG_ON_PAGE(PageTail(page), page) would be > useful to catch all the fallouts. +1 -- 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-12-22 15:47 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-09 13:02 isolate_lru_page on !head pages Michal Hocko 2015-12-09 13:02 ` Michal Hocko 2015-12-14 12:04 ` Kirill A. Shutemov 2015-12-14 12:04 ` Kirill A. Shutemov 2015-12-15 8:52 ` Michal Hocko 2015-12-15 8:52 ` Michal Hocko 2015-12-15 12:03 ` Kirill A. Shutemov 2015-12-15 12:03 ` Kirill A. Shutemov 2015-12-15 16:59 ` Michal Hocko 2015-12-15 16:59 ` Michal Hocko 2015-12-22 15:47 ` Vlastimil Babka 2015-12-22 15:47 ` Vlastimil Babka
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.