5:25 a.m.
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.
---
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;
}
--
2.33.1
8:48 p.m.
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;
> }
8:57 p.m.
Hi Kazu,
On Fri, Apr 21, 2023 at 9:48 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> wrote:
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 for the test data, and the previous suggestions which make the
patch better!
Thanks,
Tao Liu
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;
> }
9:41 p.m.
On 2023/04/21 10:57, Tao Liu wrote:
Hi Kazu,
On Fri, Apr 21, 2023 at 9:48 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> wrote:
>
> 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 for the test data, and the previous suggestions which make the
patch better!
Good to help :)
Just an idea on another optimization,
SEARCHMASK(si->s_parms.s_ulong.value[si->val])
in search_ulong() might be pre-processed somewhere.
For loops, reducing machine instructions is often effective.
Thanks,
Kazu
>
> Thanks,
> Tao Liu
>
>> 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;
>>> }
10:08 p.m.
Hi Kazu,
On Fri, Apr 21, 2023 at 10:41 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> wrote:
On 2023/04/21 10:57, Tao Liu wrote:
> Hi Kazu,
>
> On Fri, Apr 21, 2023 at 9:48 AM HAGIO KAZUHITO(萩尾 一仁)
> <k-hagio-ab(a)nec.com> wrote:
>>
>> 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 for the test data, and the previous suggestions which make the
> patch better!
Good to help :)
Just an idea on another optimization,
SEARCHMASK(si->s_parms.s_ulong.value[si->val])
in search_ulong() might be pre-processed somewhere.
For loops, reducing machine instructions is often effective.
Do you mean change the code like the following in search_ulong()?
int value = SEARCHMASK(si->s_parms.s_ulong.value[si->val]);
for (i = 0; i < longcnt; i++, bufptr++, addr += sizeof(long)) {
for (si->val = 0; si->val < si->vcnt; si->val++) {
if (SEARCHMASK(*bufptr) == value)) {
....
}
}
}
In this way, we don't need to recalculate the value every time in the loop.
Thanks,
Tao Liu
Thanks,
Kazu
>
> Thanks,
> Tao Liu
>
>> 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;
>>> }
11:09 p.m.
On 2023/04/21 12:08, Tao Liu wrote:
> Just an idea on another optimization,
>
> SEARCHMASK(si->s_parms.s_ulong.value[si->val])
>
> in search_ulong() might be pre-processed somewhere.
> For loops, reducing machine instructions is often effective.
>
Do you mean change the code like the following in search_ulong()?
int value = SEARCHMASK(si->s_parms.s_ulong.value[si->val]);
for (i = 0; i < longcnt; i++, bufptr++, addr += sizeof(long)) {
for (si->val = 0; si->val < si->vcnt; si->val++) {
if (SEARCHMASK(*bufptr) == value)) {
....
}
}
}
In this way, we don't need to recalculate the value every time in the loop.
Yes, this might be good, or the function itself is called many times so
adding a member might be good if it's effective:
struct {
ulong value[MAXARGS];
char *opt_string[MAXARGS];
ulong mask;
+ ulong masked_value[MAXARGS];
} s_ulong;
and in search_ulong(),
int *masked = si->s_parms.s_ulong.masked_value;
...
if (SEARCHMASK(*bufptr) == masked[si->val])
(Ideally, it might be better to reduce member access e.g. si->s_parms...
in the loop, because it may use a few instructions. (or gcc may optimize
it?) objdump can be used to check :)
Thanks,
Kazu
617
days inactive
618
days old
devel@lists.crash-utility.osci.io
5 comments
2 participants
participants (2)
-
HAGIO KAZUHITO(萩尾 一仁) -
Tao Liu