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@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?
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@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
