Received-SPF: pass (google.com: domain of linux-kernel+bounces-205843-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <e695bf8e-9745-41ba-9986-fc888df71d3f@gmail.com>
Date: Fri, 7 Jun 2024 11:40:39 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 1/2] mm: clear pte for folios that are zero filled
To: Shakeel Butt <shakeel.butt@linux.dev>, yosryahmed@google.com
Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, willy@infradead.org,
 nphamcs@gmail.com, chengming.zhou@linux.dev, linux-mm@kvack.org,
 linux-kernel@vger.kernel.org, kernel-team@meta.com
References: <20240604105950.1134192-1-usamaarif642@gmail.com>
 <20240604105950.1134192-2-usamaarif642@gmail.com>
 <nes73bwc5p6yhwt5tw3upxcqrn5kenn6lvqb6exrf4yppmz6jx@ywhuevpkxlvh>
Content-Language: en-US
From: Usama Arif <usamaarif642@gmail.com>
In-Reply-To: <nes73bwc5p6yhwt5tw3upxcqrn5kenn6lvqb6exrf4yppmz6jx@ywhuevpkxlvh>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 05/06/2024 09:55, Shakeel Butt wrote:
> On Tue, Jun 04, 2024 at 11:58:24AM GMT, Usama Arif wrote:
> [...]
>>   
>> +static bool is_folio_page_zero_filled(struct folio *folio, int i)
>> +{
>> +	unsigned long *data;
>> +	unsigned int pos, last_pos = PAGE_SIZE / sizeof(*data) - 1;
>> +	bool ret = false;
>> +
>> +	data = kmap_local_folio(folio, i * PAGE_SIZE);
>> +
>> +	if (data[last_pos])
>> +		goto out;
>> +
> Use memchr_inv() instead of the following.

I had done some benchmarking before sending v1 and this version is 35% 
faster than using memchr_inv(). Its likely because this does long 
comparison, while memchr_inv does a byte comparison using check_bytes8 
[1]. I will stick with the current version for my next revision. I have 
added the kernel module I used for benchmarking below:

[308797.975269] Time taken for orig: 2850 ms
[308801.911439] Time taken for memchr_inv: 3936 ms

[1] https://elixir.bootlin.com/linux/v6.9.3/source/lib/string.c#L800


#include <linux/time.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/string.h>

#define ITERATIONS 10000000
static int is_page_zero_filled(void *ptr, unsigned long *value)
{
     unsigned long *page;
     unsigned long val;
     unsigned int pos, last_pos = PAGE_SIZE / sizeof(*page) - 1;

     page = (unsigned long *)ptr;
     val = page[0];

     if (page[last_pos] != 0)
         return 0;

     for (pos = 1; pos < last_pos; pos++) {
         if (page[pos] != 0)
             return 0;
     }

     *value = val;

     return 1;
}

static int is_page_zero_filled_memchr_inv(void *ptr, unsigned long *value)
{
     unsigned long *page;
     unsigned long val;
     unsigned long *ret;
     page = (unsigned long *)ptr;

     val = page[0];
     *value = val;

     ret = memchr_inv(ptr, 0, PAGE_SIZE);

     return ret == NULL ? 1: 0;
}

static int __init zsmalloc_test_init(void)
{
     unsigned long *src;
     unsigned long value;
     ktime_t start_time, end_time;
     volatile int res = 0;
     unsigned long milliseconds;

     src = kmalloc(PAGE_SIZE, GFP_KERNEL);
     if (!src)
         return -ENOMEM;

     for (unsigned int pos = 0; pos <= PAGE_SIZE / sizeof(*src) - 1; 
pos++) {
         src[pos] = 0x0;
     }

     start_time = ktime_get();
     for (int i = 0; i < ITERATIONS; i++)
         res = is_page_zero_filled(src, &value);
     end_time = ktime_get();
     milliseconds = ktime_ms_delta(end_time, start_time);
     // printk(KERN_INFO "Result: %d, Value: %lu\n", res, value);
     printk(KERN_INFO "Time taken for orig: %lu ms\n", milliseconds);

     start_time = ktime_get();
     for (int i = 0; i < ITERATIONS; i++)
         res = is_page_zero_filled_memchr_inv(src, &value);
     end_time = ktime_get();
     milliseconds = ktime_ms_delta(end_time, start_time);
     // printk(KERN_INFO "Result: %d, Value: %lu\n", res, value);
     printk(KERN_INFO "Time taken for memchr_inv: %lu ms\n", milliseconds);

     kfree(src);
     // Dont insmod so that you can re-run
     return -1;
}

module_init(zsmalloc_test_init);
MODULE_LICENSE("GPL");