svn commit: r322272 - head/sys/compat/linuxkpi/common/src
Hans Petter Selasky
hps at selasky.org
Thu Jan 7 09:03:15 UTC 2021
On 1/6/21 9:44 PM, Ryan Stone wrote:
> On Tue, Aug 8, 2017 at 3:36 PM Alexander Motin <mav at freebsd.org> wrote:
>>
>> Author: mav
>> Date: Tue Aug 8 19:36:34 2017
>> New Revision: 322272
>> URL: https://svnweb.freebsd.org/changeset/base/322272
>>
>> Log:
>> Fix few issues of LinuxKPI workqueue.
>>
>> LinuxKPI workqueue wrappers reported "successful" cancellation for works
>> already completed in normal way. This change brings reported status and
>> real cancellation fact into sync. This required for drm-next operation.
>>
>> Reviewed by: hselasky (earlier version)
>> Sponsored by: iXsystems, Inc.
>> Differential Revision: https://reviews.freebsd.org/D11904
>>
>
>> @@ -266,13 +267,14 @@ linux_delayed_work_timer_fn(void *arg)
>> [WORK_ST_IDLE] = WORK_ST_IDLE, /* NOP */
>> [WORK_ST_TIMER] = WORK_ST_TASK, /* start queueing task */
>> [WORK_ST_TASK] = WORK_ST_TASK, /* NOP */
>> - [WORK_ST_EXEC] = WORK_ST_TASK, /* queue task another time */
>> - [WORK_ST_CANCEL] = WORK_ST_IDLE, /* complete cancel */
>> + [WORK_ST_EXEC] = WORK_ST_EXEC, /* NOP */
>> + [WORK_ST_CANCEL] = WORK_ST_TASK, /* failed to cancel */
>> };
>> struct delayed_work *dwork = arg;
>>
>> switch (linux_update_state(&dwork->work.state, states)) {
>> case WORK_ST_TIMER:
>> + case WORK_ST_CANCEL:
>> linux_delayed_work_enqueue(dwork);
>> break;
>> default:
>
> I believe that this hunk introduced a regression into workqueue.
> Consider the following scenario:
>
> 1. A delayed_work struct in the WORK_ST_TIMER state.
> 2. Thread A calls mod_delayed_work()
> 3. Thread B (a callout thread) simultaneously calls
> linux_delayed_work_timer_fn()
>
> The following sequence of events is possible:
>
> A: Call linux_cancel_delayed_work()
> A: Change state from TIMER TO CANCEL
> B: Change state from CANCEL to TASK
> B: taskqueue_enqueue() the task
> A: taskqueue_cancel() the task
> A: Call linux_queue_delayed_work_on(). This is a no-op because the
> state is WORK_ST_TASK.
>
> As a result, the delayed_work struct will never be invoked. This is
> causing address resolution in ib_addr.c to stop permanently, as it
> never tries to reschedule a task that it thinks is already scheduled.
>
> Do you have a recommendation? Should we unconditionally
> taskqueue_enqueue() when in the WORK_ST_TASK state and
> linux_queue_delayed_work_on() is called? That is harmless for a
> pending task but will break the deadlock if the race is lost.
>
Hi Ryan,
Do you have a patch to fix this?
--HPS
More information about the svn-src-all
mailing list