Re: [Crash-utility] [PATCH v2] Replace lseek/read into pread for kcore and vmcore reading.
2023/04/20 19:25, Tao Liu wrote:
Previously crash use lseek/read for kcore and vmcore reading, this
involves 2
syscalls. And we can replace them with pread, only 1 syscall is needed for
kcore/vmcore reading, and we can have a better performance. Please note there
are plenty of places in crash using lseek/read, this patch doesn't modify all
of them, just the most commonly used kcore and diskdump vmcore reading.
Signed-off-by: Tao Liu <ltao(a)redhat.com>
---
v1 -> v2: add offset check before pread.
Thanks for the v2, looks good to me.
also confirmed that 'search abcd' for 64GB /proc/kcore became faster:
real 0m56.733s real 0m47.446s
user 0m28.187s --> user 0m26.663s
sys 0m28.592s sys 0m20.834s
Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Thanks,
Kazu
>
> ---
> diskdump.c | 31 +++++++++++++++++++++++++------
> netdump.c | 19 ++++++++++++++-----
> 2 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/diskdump.c b/diskdump.c
> index cf5f5d9..6ecd08a 100644
> --- a/diskdump.c
> +++ b/diskdump.c
> @@ -515,16 +515,24 @@ arm_kdump_header_adjust(int header_version)
> static int
> read_pd(int fd, off_t offset, page_desc_t *pd)
> {
> - const off_t failed = (off_t)-1;
> + int ret;
>
> if (FLAT_FORMAT()) {
> if (!read_flattened_format(fd, offset, pd, sizeof(*pd)))
> return READ_ERROR;
> } else {
> - if (lseek(fd, offset, SEEK_SET) == failed)
> + if (offset < 0) {
> + if (CRASHDEBUG(8)) {
> + fprintf(fp, "read_pd: invalid offset: %lx\n", offset);
> + }
> return SEEK_ERROR;
> - if (read(fd, pd, sizeof(*pd)) != sizeof(*pd))
> + }
> + if ((ret = pread(fd, pd, sizeof(*pd), offset)) != sizeof(*pd)) {
> + if (ret == -1 && CRASHDEBUG(8)) {
> + fprintf(fp, "read_pd: pread error: %s\n", strerror(errno));
> + }
> return READ_ERROR;
> + }
> }
>
> return 0;
> @@ -1125,7 +1133,6 @@ cache_page(physaddr_t paddr)
> off_t seek_offset;
> page_desc_t pd;
> const int block_size = dd->block_size;
> - const off_t failed = (off_t)-1;
> ulong retlen;
> #ifdef ZSTD
> static ZSTD_DCtx *dctx = NULL;
> @@ -1190,10 +1197,22 @@ cache_page(physaddr_t paddr)
> return PAGE_INCOMPLETE;
> }
> } else {
> - if (lseek(dd->dfd, pd.offset, SEEK_SET) == failed)
> + if (pd.offset < 0) {
> + if (CRASHDEBUG(8)) {
> + fprintf(fp,
> + "read_diskdump/cache_page: "
> + "invalid offset: %lx\n", pd.offset);
> + }
> return SEEK_ERROR;
> - if (read(dd->dfd, dd->compressed_page, pd.size) != pd.size)
> + }
> + if ((ret = pread(dd->dfd, dd->compressed_page, pd.size, pd.offset)) !=
pd.size) {
> + if (ret == -1 && CRASHDEBUG(8)) {
> + fprintf(fp,
> + "read_diskdump/cache_page: "
> + "pread error: %s\n", strerror(errno));
> + }
> return READ_ERROR;
> + }
> }
>
> if (pd.flags & DUMP_DH_COMPRESSED_ZLIB) {
> diff --git a/netdump.c b/netdump.c
> index 01af145..696f5fd 100644
> --- a/netdump.c
> +++ b/netdump.c
> @@ -4336,7 +4336,7 @@ no_nt_prstatus_exists:
> int
> read_proc_kcore(int fd, void *bufptr, int cnt, ulong addr, physaddr_t paddr)
> {
> - int i;
> + int i, ret;
> size_t readcnt;
> ulong kvaddr;
> Elf32_Phdr *lp32;
> @@ -4436,11 +4436,20 @@ read_proc_kcore(int fd, void *bufptr, int cnt, ulong addr,
physaddr_t paddr)
> if (offset == UNINITIALIZED)
> return SEEK_ERROR;
>
> - if (lseek(fd, offset, SEEK_SET) != offset)
> - perror("lseek");
> -
> - if (read(fd, bufptr, readcnt) != readcnt)
> + if (offset < 0) {
> + if (CRASHDEBUG(8)) {
> + fprintf(fp, "read_proc_kcore: "
> + "invalid offset: %lx\n", offset);
> + }
> + return SEEK_ERROR;
> + }
> + if ((ret = pread(fd, bufptr, readcnt, offset)) != readcnt) {
> + if (ret == -1 && CRASHDEBUG(8)) {
> + fprintf(fp, "read_proc_kcore: "
> + "pread error: %s\n", strerror(errno));
> + }
> return READ_ERROR;
> + }
>
> return cnt;
> }