Hi Kazu,
On Fri, Apr 21, 2023 at 9:48 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> 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@xxxxxxxxxx>
> > ---
> >
> > 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@xxxxxxx>
>
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;
> > }
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
