| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2f5af934-de4a-4548-ac59-f23c3ae0f54e@ideasonboard.com>
Date: Sat, 7 Sep 2024 23:56:36 +0530
From: Umang Jain <umang.jain@...asonboard.com>
To: Stefan Wahren <wahrenst@....net>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-rpi-kernel@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-staging@...ts.linux.dev,
linux-kernel@...r.kernel.org,
Kieran Bingham <kieran.bingham@...asonboard.com>,
Arnd Bergmann <arnd@...db.de>,
Dave Stevenson <dave.stevenson@...pberrypi.com>,
Phil Elwell <phil@...pberrypi.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH v4 3/7] staging: vchiq_core: Factor out bulk transfer for
blocking mode
Hi Stefan
On 07/09/24 5:19 pm, Stefan Wahren wrote:
> Hi Umang,
>
> Am 06.09.24 um 09:25 schrieb Umang Jain:
>> Factor out bulk transfer for blocking mode into a separate dedicated
>> function bulk_xfer_blocking_interruptible(). It is suffixed by
>> "_interruptible" to denote that it can be interrupted and -EAGAIN
>> can be returned. It would be up to the users of the function to retry
>> the call in those cases.
>>
>> Adjust the calls to vchiq-dev.c ioctl interface and vchiq_arm.c
>> for blocking bulk transfers.
>>
>> Signed-off-by: Umang Jain <umang.jain@...asonboard.com>
> i applied this series on top of recent linux-next and the vchiq ping
> test on Raspberry Pi 3 B Plus (multi_v7_defconfig) crashes or hanges
> strangly.
This is really annoying. The behavior is non-deterministic
I tested this repeatedly and I see the hang on my 3rd ping test.
The first 2 ping tests completed successfully.
Here is the output for all 3 tries:
https://paste.debian.net/plain/1328739
I'll take a look and fix it in next version.
>
> I bisected the issue to this patch, but it's possible the root cause
> might be introduced before.
>
> But i'm pretty sure that the series introduced the regression.
>
> Sorry, i don't have the time analyse this issue deeper.
>
> Best regards
>> ---
>> .../interface/vchiq_arm/vchiq_arm.c | 5 +--
>> .../interface/vchiq_arm/vchiq_core.c | 42 ++++++++++++++++---
>> .../interface/vchiq_arm/vchiq_core.h | 5 +++
>> .../interface/vchiq_arm/vchiq_dev.c | 6 +++
>> 4 files changed, 49 insertions(+), 9 deletions(-)
>>
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> index c4d97dbf6ba8..688c9b1be868 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> @@ -968,9 +968,8 @@ vchiq_blocking_bulk_transfer(struct
>> vchiq_instance *instance, unsigned int handl
>> return -ENOMEM;
>> }
>>
>> - ret = vchiq_bulk_transfer(instance, handle, data, NULL, size,
>> - &waiter->bulk_waiter,
>> - VCHIQ_BULK_MODE_BLOCKING, dir);
>> + ret = vchiq_bulk_xfer_blocking_interruptible(instance, handle,
>> data, NULL, size,
>> + &waiter->bulk_waiter, dir);
>> if ((ret != -EAGAIN) || fatal_signal_pending(current) ||
>> !waiter->bulk_waiter.bulk) {
>> struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;
>>
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> index f36044bab194..43f951fa4b89 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> @@ -2985,6 +2985,42 @@ vchiq_remove_service(struct vchiq_instance
>> *instance, unsigned int handle)
>> return status;
>> }
>>
>> +int
>> +vchiq_bulk_xfer_blocking_interruptible(struct vchiq_instance
>> *instance, unsigned int handle,
>> + void *offset, void __user *uoffset, int size,
>> + void __user *userdata, enum vchiq_bulk_dir dir)
>> +{
>> + struct vchiq_service *service = find_service_by_handle(instance,
>> handle);
>> + struct bulk_waiter *bulk_waiter;
>> + enum vchiq_bulk_mode mode = VCHIQ_BULK_MODE_BLOCKING;
>> + int status = -EINVAL;
>> +
>> + if (!service)
>> + return -EINVAL;
>> +
>> + if (service->srvstate != VCHIQ_SRVSTATE_OPEN)
>> + goto error_exit;
>> +
>> + if (!offset && !uoffset)
>> + goto error_exit;
>> +
>> + if (vchiq_check_service(service))
>> + goto error_exit;
>> +
>> + bulk_waiter = userdata;
>> + init_completion(&bulk_waiter->event);
>> + bulk_waiter->actual = 0;
>> + bulk_waiter->bulk = NULL;
>> +
>> + status = vchiq_bulk_xfer_queue_msg_interruptible(service,
>> offset, uoffset, size,
>> + userdata, mode, dir);
>> +
>> +error_exit:
>> + vchiq_service_put(service);
>> +
>> + return status;
>> +}
>> +
>> /*
>> * This function may be called by kernel threads or user threads.
>> * User threads may receive -EAGAIN to indicate that a signal has been
>> @@ -3018,12 +3054,6 @@ int vchiq_bulk_transfer(struct vchiq_instance
>> *instance, unsigned int handle,
>> case VCHIQ_BULK_MODE_NOCALLBACK:
>> case VCHIQ_BULK_MODE_CALLBACK:
>> break;
>> - case VCHIQ_BULK_MODE_BLOCKING:
>> - bulk_waiter = userdata;
>> - init_completion(&bulk_waiter->event);
>> - bulk_waiter->actual = 0;
>> - bulk_waiter->bulk = NULL;
>> - break;
>> default:
>> goto error_exit;
>> }
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> index 985d9ea3a06a..2dd89101c1c6 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
>> @@ -474,6 +474,11 @@ extern int
>> vchiq_bulk_xfer_waiting_interruptible(struct vchiq_instance *instance,
>> unsigned int handle, struct bulk_waiter
>> *userdata);
>>
>> +extern int
>> +vchiq_bulk_xfer_blocking_interruptible(struct vchiq_instance
>> *instance, unsigned int handle,
>> + void *offset, void __user *uoffset, int size,
>> + void __user *userdata, enum vchiq_bulk_dir dir);
>> +
>> extern int
>> vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int
>> handle, void *offset,
>> void __user *uoffset, int size, void *userdata, enum
>> vchiq_bulk_mode mode,
>> diff --git
>> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> index 550838d2863b..830633f2326b 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>> @@ -304,6 +304,12 @@ static int vchiq_irq_queue_bulk_tx_rx(struct
>> vchiq_instance *instance,
>> }
>>
>> userdata = &waiter->bulk_waiter;
>> +
>> + status = vchiq_bulk_xfer_blocking_interruptible(instance,
>> args->handle,
>> + NULL, args->data, args->size,
>> + userdata, dir);
>> +
>> + goto bulk_transfer_handled;
>> } else if (args->mode == VCHIQ_BULK_MODE_WAITING) {
>> mutex_lock(&instance->bulk_waiter_list_mutex);
>> list_for_each_entry(iter, &instance->bulk_waiter_list,
>
Powered by blists - more mailing lists