Re: [Crash-utility] [PATCH 5/5] [RFC] Replace lseek/read into pread for kcore and vmcore reading.
On 2023/04/11 10:23, Tao Liu wrote:
Hi Kazu,
Sorry for the late reply.
No problem :-)
On Fri, Apr 7, 2023 at 10:36 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> 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?
No, I would like to know which causes an error in pread(), seek or read.
In other words, is there any way to know which is wrong, the specified
offset or file/media?
I thought that the current crash can distinguish them and it would be
better to keep the behavior for debugging, if possible.
Hope this helps..
Thanks,
Kazu
>
> Thanks,
> Tao Liu
>
>> Thanks,
>> Kazu
>>
>>>
>>> Signed-off-by: Tao Liu <ltao(a)redhat.com>
>>> ---
>>> 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;