| 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: <c7e302b6-fc62-4754-ab1d-7c2771cccf60@gmx.net>
Date: Sun, 8 Sep 2024 12:41:06 +0200
From: Stefan Wahren <wahrenst@....net>
To: Umang Jain <umang.jain@...asonboard.com>,
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 Umang,
Am 07.09.24 um 20:26 schrieb Umang Jain:
> 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 was too imprecise with my information.
Just try "vchiq_test -p" which makes a big difference in reproducibility.
Best regards
>
>>
>> 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