Hi Kazu,
Sorry for the late reply.
On Fri, Apr 7, 2023 at 10:36 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> wrote:
>
> On 2023/03/25 13:12, 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.
>
> This is nice as an optimization, but can we distinguish a seek error and
> read error with errno as currently we can do?
>
Sorry I didn't understand your question...
The functions which I modified in this patch didn't use a global errno
to distinguish error types, they used a returned value instead.
Currently if read error, READ_ERROR will be returned, same as seek
error returns SEEK_ERROR. Do you want to distinguish pread error by
introducing a new error number as PREAD_ERROR?
Thanks,
Tao Liu
> Thanks,
> Kazu
>
> >
> > Signed-off-by: Tao Liu <ltao@xxxxxxxxxx>
> > ---
> > diskdump.c | 11 ++---------
> > netdump.c | 5 +----
> > 2 files changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/diskdump.c b/diskdump.c
> > index cf5f5d9..acf79f2 100644
> > --- a/diskdump.c
> > +++ b/diskdump.c
> > @@ -515,15 +515,11 @@ 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;
> > -
> > if (FLAT_FORMAT()) {
> > if (!read_flattened_format(fd, offset, pd, sizeof(*pd)))
> > return READ_ERROR;
> > } else {
> > - if (lseek(fd, offset, SEEK_SET) == failed)
> > - return SEEK_ERROR;
> > - if (read(fd, pd, sizeof(*pd)) != sizeof(*pd))
> > + if (pread(fd, pd, sizeof(*pd), offset) != sizeof(*pd))
> > return READ_ERROR;
> > }
> >
> > @@ -1125,7 +1121,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,9 +1185,7 @@ cache_page(physaddr_t paddr)
> > return PAGE_INCOMPLETE;
> > }
> > } else {
> > - if (lseek(dd->dfd, pd.offset, SEEK_SET) == failed)
> > - return SEEK_ERROR;
> > - if (read(dd->dfd, dd->compressed_page, pd.size) != pd.size)
> > + if (pread(dd->dfd, dd->compressed_page, pd.size, pd.offset) != pd.size)
> > return READ_ERROR;
> > }
> >
> > diff --git a/netdump.c b/netdump.c
> > index 01af145..e76807a 100644
> > --- a/netdump.c
> > +++ b/netdump.c
> > @@ -4436,10 +4436,7 @@ 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 (pread(fd, bufptr, readcnt, offset) != readcnt)
> > 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
