* Re: [PATCH] mm: reposition zbud page in lru list if not freed in zbud_free
[not found] ` <20140902143936.GA11096@cerebellum.variantweb.net>
@ 2014-09-02 23:59 ` Wang Sheng-Hui
2014-09-03 0:06 ` Wang Sheng-Hui
1 sibling, 0 replies; 3+ messages in thread
From: Wang Sheng-Hui @ 2014-09-02 23:59 UTC (permalink / raw)
To: Seth Jennings; +Cc: linux-mm
On 2014a1'09ae??02ae?JPY 22:39, Seth Jennings wrote:
> On Sun, Aug 31, 2014 at 09:29:29PM +0800, Wang Sheng-Hui wrote:
>> Reposition zbud page in the lru list of the pool if the zbud page
>> is not freed in zbud_free.
>
> This doesn't mention what is wrong with the current code. Afaict, the
> code is doing the right thing by not changing the position of the zpage
> in the LRU. Why would we want to move a zpage to the front of the LRU
> just because one of the allocations it contains is freed? The "age" of
> the other allocation is unchanged and its containing zpage should
> maintain its position in the LRU, right?
>
Thanks for your explanation, Seth!
Regards,
Sheng-Hui
> Thanks,
> Seth
>
>>
>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
>> ---
>> mm/zbud.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/zbud.c b/mm/zbud.c
>> index f26e7fc..b1d7777 100644
>> --- a/mm/zbud.c
>> +++ b/mm/zbud.c
>> @@ -432,15 +432,16 @@ void zbud_free(struct zbud_pool *pool, unsigned long handle)
>> /* Remove from existing buddy list */
>> list_del(&zhdr->buddy);
>>
>> + list_del(&zhdr->lru);
>> if (zhdr->first_chunks == 0 && zhdr->last_chunks == 0) {
>> /* zbud page is empty, free */
>> - list_del(&zhdr->lru);
>> free_zbud_page(zhdr);
>> pool->pages_nr--;
>> } else {
>> /* Add to unbuddied list */
>> freechunks = num_free_chunks(zhdr);
>> list_add(&zhdr->buddy, &pool->unbuddied[freechunks]);
>> + list_add(&zhdr->lru, &pool->lru);
>> }
>>
>> spin_unlock(&pool->lock);
>> --
>> 1.8.3.2
>>
--
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] 3+ messages in thread* Re: [PATCH] mm: reposition zbud page in lru list if not freed in zbud_free
[not found] ` <20140902143936.GA11096@cerebellum.variantweb.net>
2014-09-02 23:59 ` Wang Sheng-Hui
@ 2014-09-03 0:06 ` Wang Sheng-Hui
1 sibling, 0 replies; 3+ messages in thread
From: Wang Sheng-Hui @ 2014-09-03 0:06 UTC (permalink / raw)
To: Seth Jennings; +Cc: linux-mm
On 2014a1'09ae??02ae?JPY 22:39, Seth Jennings wrote:
> On Sun, Aug 31, 2014 at 09:29:29PM +0800, Wang Sheng-Hui wrote:
>> Reposition zbud page in the lru list of the pool if the zbud page
>> is not freed in zbud_free.
>
> This doesn't mention what is wrong with the current code. Afaict, the
> code is doing the right thing by not changing the position of the zpage
> in the LRU. Why would we want to move a zpage to the front of the LRU
> just because one of the allocations it contains is freed? The "age" of
> the other allocation is unchanged and its containing zpage should
> maintain its position in the LRU, right?
>
Seth,
I have one question.
In zbud_reclaim_page, if one page contains handles not freed, then the page
is repositioned, in current code.
540 } else if (zhdr->first_chunks == 0 ||
541 zhdr->last_chunks == 0) {
542 /* add to unbuddied list */
543 freechunks = num_free_chunks(zhdr);
544 list_add(&zhdr->buddy, &pool->unbuddied[freechunks]);
545 } else {
546 /* add to buddied list */
547 list_add(&zhdr->buddy, &pool->buddied);
548 }
549
550 /* add to beginning of LRU */
551 list_add(&zhdr->lru, &pool->lru);
I want to why it is different for zbud_free?
Regards,
Sheng-Hui
> Thanks,
> Seth
>
>>
>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
>> ---
>> mm/zbud.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/zbud.c b/mm/zbud.c
>> index f26e7fc..b1d7777 100644
>> --- a/mm/zbud.c
>> +++ b/mm/zbud.c
>> @@ -432,15 +432,16 @@ void zbud_free(struct zbud_pool *pool, unsigned long handle)
>> /* Remove from existing buddy list */
>> list_del(&zhdr->buddy);
>>
>> + list_del(&zhdr->lru);
>> if (zhdr->first_chunks == 0 && zhdr->last_chunks == 0) {
>> /* zbud page is empty, free */
>> - list_del(&zhdr->lru);
>> free_zbud_page(zhdr);
>> pool->pages_nr--;
>> } else {
>> /* Add to unbuddied list */
>> freechunks = num_free_chunks(zhdr);
>> list_add(&zhdr->buddy, &pool->unbuddied[freechunks]);
>> + list_add(&zhdr->lru, &pool->lru);
>> }
>>
>> spin_unlock(&pool->lock);
>> --
>> 1.8.3.2
>>
--
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] 3+ messages in thread