* [PATCH] selftests: get readahead size for check_file_mmap()
@ 2021-03-30 12:40 Jeffle Xu
2021-03-30 12:45 ` JeffleXu
0 siblings, 1 reply; 3+ messages in thread
From: Jeffle Xu @ 2021-03-30 12:40 UTC (permalink / raw)
To: ricardo.canuelo, shuah; +Cc: linux-kselftest, Jeffle Xu
The readahead size used to be 2MB, thus it's reasonable to set the file
size as 4MB when checking check_file_mmap().
However since commit c2e4cd57cfa1 ("block: lift setting the readahead
size into the block layer"), readahead could be as large as twice the
io_opt, and thus the hardcoded file size no longer works.
Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
.../selftests/mincore/mincore_selftest.c | 57 +++++++++++++++----
1 file changed, 47 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/mincore/mincore_selftest.c b/tools/testing/selftests/mincore/mincore_selftest.c
index 5a1e85ff5d32..cf0c86697403 100644
--- a/tools/testing/selftests/mincore/mincore_selftest.c
+++ b/tools/testing/selftests/mincore/mincore_selftest.c
@@ -15,6 +15,11 @@
#include <string.h>
#include <fcntl.h>
#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/sysmacros.h>
+#include <sys/mount.h>
#include "../kselftest.h"
#include "../kselftest_harness.h"
@@ -195,10 +200,42 @@ TEST(check_file_mmap)
int fd;
int i;
int ra_pages = 0;
+ long ra_size, filesize;
+ struct stat stats;
+ dev_t devt;
+ unsigned int major, minor;
+ char devpath[32];
+
+ retval = stat(".", &stats);
+ ASSERT_EQ(0, retval) {
+ TH_LOG("Can't stat pwd: %s", strerror(errno));
+ }
+
+ devt = stats.st_dev;
+ major = major(devt);
+ minor = minor(devt);
+ snprintf(devpath, sizeof(devpath), "/dev/block/%u:%u", major, minor);
+
+ fd = open(devpath, O_RDONLY);
+ ASSERT_NE(-1, fd) {
+ TH_LOG("Can't open underlying disk %s", strerror(errno));
+ }
+
+ retval = ioctl(fd, BLKRAGET, &ra_size);
+ ASSERT_EQ(0, retval) {
+ TH_LOG("Error ioctl with the underlying disk: %s", strerror(errno));
+ }
+
+ /*
+ * BLKRAGET ioctl returns the readahead size in sectors (512 bytes).
+ * Make filesize large enough to contain the readahead window.
+ */
+ ra_size *= 512;
+ filesize = ra_size * 2;
page_size = sysconf(_SC_PAGESIZE);
- vec_size = FILE_SIZE / page_size;
- if (FILE_SIZE % page_size)
+ vec_size = filesize / page_size;
+ if (filesize % page_size)
vec_size++;
vec = calloc(vec_size, sizeof(unsigned char));
@@ -213,7 +250,7 @@ TEST(check_file_mmap)
strerror(errno));
}
errno = 0;
- retval = fallocate(fd, 0, 0, FILE_SIZE);
+ retval = fallocate(fd, 0, 0, filesize);
ASSERT_EQ(0, retval) {
TH_LOG("Error allocating space for the temporary file: %s",
strerror(errno));
@@ -223,12 +260,12 @@ TEST(check_file_mmap)
* Map the whole file, the pages shouldn't be fetched yet.
*/
errno = 0;
- addr = mmap(NULL, FILE_SIZE, PROT_READ | PROT_WRITE,
+ addr = mmap(NULL, filesize, PROT_READ | PROT_WRITE,
MAP_SHARED, fd, 0);
ASSERT_NE(MAP_FAILED, addr) {
TH_LOG("mmap error: %s", strerror(errno));
}
- retval = mincore(addr, FILE_SIZE, vec);
+ retval = mincore(addr, filesize, vec);
ASSERT_EQ(0, retval);
for (i = 0; i < vec_size; i++) {
ASSERT_EQ(0, vec[i]) {
@@ -240,14 +277,14 @@ TEST(check_file_mmap)
* Touch a page in the middle of the mapping. We expect the next
* few pages (the readahead window) to be populated too.
*/
- addr[FILE_SIZE / 2] = 1;
- retval = mincore(addr, FILE_SIZE, vec);
+ addr[filesize / 2] = 1;
+ retval = mincore(addr, filesize, vec);
ASSERT_EQ(0, retval);
- ASSERT_EQ(1, vec[FILE_SIZE / 2 / page_size]) {
+ ASSERT_EQ(1, vec[filesize / 2 / page_size]) {
TH_LOG("Page not found in memory after use");
}
- i = FILE_SIZE / 2 / page_size + 1;
+ i = filesize / 2 / page_size + 1;
while (i < vec_size && vec[i]) {
ra_pages++;
i++;
@@ -271,7 +308,7 @@ TEST(check_file_mmap)
}
}
- munmap(addr, FILE_SIZE);
+ munmap(addr, filesize);
close(fd);
free(vec);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] selftests: get readahead size for check_file_mmap()
2021-03-30 12:40 [PATCH] selftests: get readahead size for check_file_mmap() Jeffle Xu
@ 2021-03-30 12:45 ` JeffleXu
2021-04-02 22:07 ` Shuah Khan
0 siblings, 1 reply; 3+ messages in thread
From: JeffleXu @ 2021-03-30 12:45 UTC (permalink / raw)
To: ricardo.canuelo, shuah; +Cc: linux-kselftest
On 3/30/21 8:40 PM, Jeffle Xu wrote:
> The readahead size used to be 2MB, thus it's reasonable to set the file
> size as 4MB when checking check_file_mmap().
>
> However since commit c2e4cd57cfa1 ("block: lift setting the readahead
> size into the block layer"), readahead could be as large as twice the
> io_opt, and thus the hardcoded file size no longer works.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
Forgot to mention that otherwise, "Read-ahead pages reached the end of
the file" is reported in check_file_mmap().
> ---
> .../selftests/mincore/mincore_selftest.c | 57 +++++++++++++++----
> 1 file changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/mincore/mincore_selftest.c b/tools/testing/selftests/mincore/mincore_selftest.c
> index 5a1e85ff5d32..cf0c86697403 100644
> --- a/tools/testing/selftests/mincore/mincore_selftest.c
> +++ b/tools/testing/selftests/mincore/mincore_selftest.c
> @@ -15,6 +15,11 @@
> #include <string.h>
> #include <fcntl.h>
> #include <string.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <sys/sysmacros.h>
> +#include <sys/mount.h>
>
> #include "../kselftest.h"
> #include "../kselftest_harness.h"
> @@ -195,10 +200,42 @@ TEST(check_file_mmap)
> int fd;
> int i;
> int ra_pages = 0;
> + long ra_size, filesize;
> + struct stat stats;
> + dev_t devt;
> + unsigned int major, minor;
> + char devpath[32];
> +
> + retval = stat(".", &stats);
> + ASSERT_EQ(0, retval) {
> + TH_LOG("Can't stat pwd: %s", strerror(errno));
> + }
> +
> + devt = stats.st_dev;
> + major = major(devt);
> + minor = minor(devt);
> + snprintf(devpath, sizeof(devpath), "/dev/block/%u:%u", major, minor);
> +
> + fd = open(devpath, O_RDONLY);
> + ASSERT_NE(-1, fd) {
> + TH_LOG("Can't open underlying disk %s", strerror(errno));
> + }
> +
> + retval = ioctl(fd, BLKRAGET, &ra_size);
> + ASSERT_EQ(0, retval) {
> + TH_LOG("Error ioctl with the underlying disk: %s", strerror(errno));
> + }
> +
> + /*
> + * BLKRAGET ioctl returns the readahead size in sectors (512 bytes).
> + * Make filesize large enough to contain the readahead window.
> + */
> + ra_size *= 512;
> + filesize = ra_size * 2;
>
> page_size = sysconf(_SC_PAGESIZE);
> - vec_size = FILE_SIZE / page_size;
> - if (FILE_SIZE % page_size)
> + vec_size = filesize / page_size;
> + if (filesize % page_size)
> vec_size++;
>
> vec = calloc(vec_size, sizeof(unsigned char));
> @@ -213,7 +250,7 @@ TEST(check_file_mmap)
> strerror(errno));
> }
> errno = 0;
> - retval = fallocate(fd, 0, 0, FILE_SIZE);
> + retval = fallocate(fd, 0, 0, filesize);
> ASSERT_EQ(0, retval) {
> TH_LOG("Error allocating space for the temporary file: %s",
> strerror(errno));
> @@ -223,12 +260,12 @@ TEST(check_file_mmap)
> * Map the whole file, the pages shouldn't be fetched yet.
> */
> errno = 0;
> - addr = mmap(NULL, FILE_SIZE, PROT_READ | PROT_WRITE,
> + addr = mmap(NULL, filesize, PROT_READ | PROT_WRITE,
> MAP_SHARED, fd, 0);
> ASSERT_NE(MAP_FAILED, addr) {
> TH_LOG("mmap error: %s", strerror(errno));
> }
> - retval = mincore(addr, FILE_SIZE, vec);
> + retval = mincore(addr, filesize, vec);
> ASSERT_EQ(0, retval);
> for (i = 0; i < vec_size; i++) {
> ASSERT_EQ(0, vec[i]) {
> @@ -240,14 +277,14 @@ TEST(check_file_mmap)
> * Touch a page in the middle of the mapping. We expect the next
> * few pages (the readahead window) to be populated too.
> */
> - addr[FILE_SIZE / 2] = 1;
> - retval = mincore(addr, FILE_SIZE, vec);
> + addr[filesize / 2] = 1;
> + retval = mincore(addr, filesize, vec);
> ASSERT_EQ(0, retval);
> - ASSERT_EQ(1, vec[FILE_SIZE / 2 / page_size]) {
> + ASSERT_EQ(1, vec[filesize / 2 / page_size]) {
> TH_LOG("Page not found in memory after use");
> }
>
> - i = FILE_SIZE / 2 / page_size + 1;
> + i = filesize / 2 / page_size + 1;
> while (i < vec_size && vec[i]) {
> ra_pages++;
> i++;
> @@ -271,7 +308,7 @@ TEST(check_file_mmap)
> }
> }
>
> - munmap(addr, FILE_SIZE);
> + munmap(addr, filesize);
> close(fd);
> free(vec);
> }
>
--
Thanks,
Jeffle
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] selftests: get readahead size for check_file_mmap()
2021-03-30 12:45 ` JeffleXu
@ 2021-04-02 22:07 ` Shuah Khan
0 siblings, 0 replies; 3+ messages in thread
From: Shuah Khan @ 2021-04-02 22:07 UTC (permalink / raw)
To: JeffleXu, ricardo.canuelo, shuah; +Cc: linux-kselftest, Shuah Khan
On 3/30/21 6:45 AM, JeffleXu wrote:
>
>
> On 3/30/21 8:40 PM, Jeffle Xu wrote:
>> The readahead size used to be 2MB, thus it's reasonable to set the file
>> size as 4MB when checking check_file_mmap().
>>
>> However since commit c2e4cd57cfa1 ("block: lift setting the readahead
>> size into the block layer"), readahead could be as large as twice the
>> io_opt, and thus the hardcoded file size no longer works.
>>
>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>
> Forgot to mention that otherwise, "Read-ahead pages reached the end of
> the file" is reported in check_file_mmap().
>
Please update the change log and send v2 including the change history.
>> ---
>> .../selftests/mincore/mincore_selftest.c | 57 +++++++++++++++----
>> 1 file changed, 47 insertions(+), 10 deletions(-)
>>
Please include test name in the subject line
>> diff --git a/tools/testing/selftests/mincore/mincore_selftest.c b/tools/testing/selftests/mincore/mincore_selftest.c
>> index 5a1e85ff5d32..cf0c86697403 100644
>> --- a/tools/testing/selftests/mincore/mincore_selftest.c
>> +++ b/tools/testing/selftests/mincore/mincore_selftest.c
>> @@ -15,6 +15,11 @@
>> #include <string.h>
>> #include <fcntl.h>
>> #include <string.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/sysmacros.h>
>> +#include <sys/mount.h>
>>
>> #include "../kselftest.h"
>> #include "../kselftest_harness.h"
>> @@ -195,10 +200,42 @@ TEST(check_file_mmap)
>> int fd;
>> int i;
>> int ra_pages = 0;
>> + long ra_size, filesize;
>> + struct stat stats;
>> + dev_t devt;
>> + unsigned int major, minor;
>> + char devpath[32];
>> +
>> + retval = stat(".", &stats);
>> + ASSERT_EQ(0, retval) {
>> + TH_LOG("Can't stat pwd: %s", strerror(errno));
>> + }
>> +
>> + devt = stats.st_dev;
>> + major = major(devt);
>> + minor = minor(devt);
>> + snprintf(devpath, sizeof(devpath), "/dev/block/%u:%u", major, minor);
>> +
>> + fd = open(devpath, O_RDONLY);
>> + ASSERT_NE(-1, fd) {
>> + TH_LOG("Can't open underlying disk %s", strerror(errno));
>> + }
>> +
>> + retval = ioctl(fd, BLKRAGET, &ra_size);
>> + ASSERT_EQ(0, retval) {
>> + TH_LOG("Error ioctl with the underlying disk: %s", strerror(errno));
>> + }
>> +
>> + /*
>> + * BLKRAGET ioctl returns the readahead size in sectors (512 bytes).
>> + * Make filesize large enough to contain the readahead window.
>> + */
>> + ra_size *= 512;
>> + filesize = ra_size * 2;
>>
>> page_size = sysconf(_SC_PAGESIZE);
>> - vec_size = FILE_SIZE / page_size;
>> - if (FILE_SIZE % page_size)
>> + vec_size = filesize / page_size;
>> + if (filesize % page_size)
>> vec_size++;
>>
>> vec = calloc(vec_size, sizeof(unsigned char));
>> @@ -213,7 +250,7 @@ TEST(check_file_mmap)
>> strerror(errno));
>> }
>> errno = 0;
>> - retval = fallocate(fd, 0, 0, FILE_SIZE);
>> + retval = fallocate(fd, 0, 0, filesize);
>> ASSERT_EQ(0, retval) {
>> TH_LOG("Error allocating space for the temporary file: %s",
>> strerror(errno));
>> @@ -223,12 +260,12 @@ TEST(check_file_mmap)
>> * Map the whole file, the pages shouldn't be fetched yet.
>> */
>> errno = 0;
>> - addr = mmap(NULL, FILE_SIZE, PROT_READ | PROT_WRITE,
>> + addr = mmap(NULL, filesize, PROT_READ | PROT_WRITE,
>> MAP_SHARED, fd, 0);
>> ASSERT_NE(MAP_FAILED, addr) {
>> TH_LOG("mmap error: %s", strerror(errno));
>> }
>> - retval = mincore(addr, FILE_SIZE, vec);
>> + retval = mincore(addr, filesize, vec);
>> ASSERT_EQ(0, retval);
>> for (i = 0; i < vec_size; i++) {
>> ASSERT_EQ(0, vec[i]) {
>> @@ -240,14 +277,14 @@ TEST(check_file_mmap)
>> * Touch a page in the middle of the mapping. We expect the next
>> * few pages (the readahead window) to be populated too.
>> */
>> - addr[FILE_SIZE / 2] = 1;
>> - retval = mincore(addr, FILE_SIZE, vec);
>> + addr[filesize / 2] = 1;
>> + retval = mincore(addr, filesize, vec);
>> ASSERT_EQ(0, retval);
>> - ASSERT_EQ(1, vec[FILE_SIZE / 2 / page_size]) {
>> + ASSERT_EQ(1, vec[filesize / 2 / page_size]) {
>> TH_LOG("Page not found in memory after use");
>> }
>>
>> - i = FILE_SIZE / 2 / page_size + 1;
>> + i = filesize / 2 / page_size + 1;
>> while (i < vec_size && vec[i]) {
>> ra_pages++;
>> i++;
>> @@ -271,7 +308,7 @@ TEST(check_file_mmap)
>> }
>> }
>>
>> - munmap(addr, FILE_SIZE);
>> + munmap(addr, filesize);
>> close(fd);
>> free(vec);
>> }
>>
>
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-02 22:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 12:40 [PATCH] selftests: get readahead size for check_file_mmap() Jeffle Xu
2021-03-30 12:45 ` JeffleXu
2021-04-02 22:07 ` Shuah Khan
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.