On 2023/04/21 17:13, lijiang wrote:
> On Thu, Apr 20, 2023 at 8:00 PM <crash-utility-request@xxxxxxxxxx> wrote:
>
>> Date: Thu, 20 Apr 2023 18:25:22 +0800
>> From: Tao Liu <ltao@xxxxxxxxxx>
>> To: crash-utility@xxxxxxxxxx
>> Subject: [PATCH v2] Replace lseek/read into pread for
>> kcore and vmcore reading.
>> Message-ID: <20230420102521.33698-1-ltao@xxxxxxxxxx>
>> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>>
>> 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.
>>
>> ---
>> 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;
>> }
>>
>
> Thank you for the update, Tao.
>
> The v2 is expected. So: Ack
Applied with several style changes mainly for message grep:
https://github.com/crash-utility/crash/commit/2c7310aa7c5d8c1531209d720de29bb2b20767e8
Thanks,
Kazu
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
