From: JeffleXu <jefflexu@linux.alibaba.com>
To: ricardo.canuelo@collabora.com, shuah@kernel.org
Cc: linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2] selftests/mincore: get readahead size for check_file_mmap()
Date: Mon, 12 Apr 2021 11:10:46 +0800 [thread overview]
Message-ID: <35798587-128d-0b91-f4bf-7eb6e35a6bbc@linux.alibaba.com> (raw)
In-Reply-To: <20210403063800.56278-1-jefflexu@linux.alibaba.com>
ping...
On 4/3/21 2:38 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 size could be as large as twice
> the io_opt, and thus the hardcoded file size no longer works.
> check_file_mmap() may report "Read-ahead pages reached the end of the
> file" when the readahead size actually exceeds the file size in this
> case.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> chnages since v1:
> - add the test name "mincore" in the subject line
> - add the error message in commit message
> - rename @filesize to @file_size to keep a more consistent naming
> convention
> ---
> .../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..3ca542934080 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, file_size;
> + 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 file_size large enough to contain the readahead window.
> + */
> + ra_size *= 512;
> + file_size = ra_size * 2;
>
> page_size = sysconf(_SC_PAGESIZE);
> - vec_size = FILE_SIZE / page_size;
> - if (FILE_SIZE % page_size)
> + vec_size = file_size / page_size;
> + if (file_size % 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, file_size);
> 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, file_size, 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, file_size, 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[file_size / 2] = 1;
> + retval = mincore(addr, file_size, vec);
> ASSERT_EQ(0, retval);
> - ASSERT_EQ(1, vec[FILE_SIZE / 2 / page_size]) {
> + ASSERT_EQ(1, vec[file_size / 2 / page_size]) {
> TH_LOG("Page not found in memory after use");
> }
>
> - i = FILE_SIZE / 2 / page_size + 1;
> + i = file_size / 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, file_size);
> close(fd);
> free(vec);
> }
>
--
Thanks,
Jeffle
next prev parent reply other threads:[~2021-04-12 3:10 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-03 6:38 [PATCH v2] selftests/mincore: get readahead size for check_file_mmap() Jeffle Xu
2021-04-12 3:10 ` JeffleXu [this message]
2021-04-12 3:46 ` [PATCH v3] " Jeffle Xu
2021-04-12 7:15 ` Ricardo Cañuelo
2021-04-14 9:17 ` JeffleXu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=35798587-128d-0b91-f4bf-7eb6e35a6bbc@linux.alibaba.com \
--to=jefflexu@linux.alibaba.com \
--cc=linux-kselftest@vger.kernel.org \
--cc=ricardo.canuelo@collabora.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).