From owner-svn-src-head@freebsd.org Wed Jan 6 20:45:05 2021 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 0CC5D4DEC38; Wed, 6 Jan 2021 20:45:05 +0000 (UTC) (envelope-from rysto32@gmail.com) Received: from mail-pl1-x635.google.com (mail-pl1-x635.google.com [IPv6:2607:f8b0:4864:20::635]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DB1Zm01wVz3GwH; Wed, 6 Jan 2021 20:45:03 +0000 (UTC) (envelope-from rysto32@gmail.com) Received: by mail-pl1-x635.google.com with SMTP id j1so2164005pld.3; Wed, 06 Jan 2021 12:45:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KQTyIOoTa7QinfSEIgT6WSLh5SJOmI2Z5nnUGkvOUDQ=; b=D3eyL2MrE7lDjjf4fhklcnLEKJWyGd5NBIxxxzJkw0itDy5BYMgFG+8aOyweT6zS4i 5Ym86CZITb8fKp/NEeMQfSz8Ccho4a6LdRLpXL/2xbG+wOk27j9BGv3gMrNk6giY77XN 75id3yllSa6Ug5NmnMYPgQ5RpuO17ntIEyxKWg5qxpIZDvM9TwcjEpX+wUF+49PWGOJX NfioM5mikzp3G38OxxIEay97b+wHjQAJnRkh9FX2adU+qGy1Adg6kw/kiQy97MlUKrHU AxfRNt5u7p1fCy2mEWlNsEUv0XvmWMIo3dQC1dRqxNZ84++hyErwKIwNdSlZQj0USUss 9E+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KQTyIOoTa7QinfSEIgT6WSLh5SJOmI2Z5nnUGkvOUDQ=; b=jV+ifUoiFgQ59X8hJzwriRwXF5R4azKedVX6MW+Itv8Iw38q/piiUz3UStcPz8rCVL o8Uw2szbI98cTZwR2y+LWk/ykcLPzxG//FQPMDvI7uZ/9qNAN/nzfwO+/TpzXb06HDQ9 G0sE/B8lWMkypQ36yIqFrJUVCQcrw9hRhmq2Rmuu+b12ZzIhPWeer2LGyrzsun0lWYur 7rco/RvVnk1BLa58QXx6M/PqMxjGQUDB1+9csl+YpNtjFBlK0H+1kpjL9v/Uj3VgZbEC ZNNrR49WbRftPO9sHbKL7yRbjAsFUSKr5ZNiJdCzLEfneFSLaQWw+ipQ0ZbLj0PXEURh Fy+Q== X-Gm-Message-State: AOAM532mcXmhp2+YF3Rzv5AyqP8dSimgh8M9UA74ScH7DN13Kv0zj9M6 NZXy4OBEp/Mc0DKvMy7li8/+jNhIDoo/PzRS0reejAqQkGeeag== X-Google-Smtp-Source: ABdhPJwF0LLeQoKS7TznjmIlQJB1XG+lihDrKsiOddnURmqu9JEyjCvAD2lKFckgpL0qJtXJez8IqU+2AHCmyZwejr0= X-Received: by 2002:a17:902:b416:b029:dc:3657:9265 with SMTP id x22-20020a170902b416b02900dc36579265mr5975431plr.24.1609965901995; Wed, 06 Jan 2021 12:45:01 -0800 (PST) MIME-Version: 1.0 References: <201708081936.v78JaY4m074051@repo.freebsd.org> In-Reply-To: <201708081936.v78JaY4m074051@repo.freebsd.org> From: Ryan Stone Date: Wed, 6 Jan 2021 15:44:51 -0500 Message-ID: Subject: Re: svn commit: r322272 - head/sys/compat/linuxkpi/common/src To: Alexander Motin Cc: src-committers , svn-src-all , svn-src-head Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4DB1Zm01wVz3GwH X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20161025 header.b=D3eyL2Mr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of rysto32@gmail.com designates 2607:f8b0:4864:20::635 as permitted sender) smtp.mailfrom=rysto32@gmail.com X-Spamd-Result: default: False [-0.57 / 15.00]; ARC_NA(0.00)[]; RBL_DBL_DONT_QUERY_IPS(0.00)[2607:f8b0:4864:20::635:from]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20161025]; RCVD_TLS_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36:c]; FREEMAIL_FROM(0.00)[gmail.com]; MIME_GOOD(-0.10)[text/plain]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_SPAM_SHORT(0.43)[0.430]; SPAMHAUS_ZRD(0.00)[2607:f8b0:4864:20::635:from:127.0.2.255]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::635:from]; NEURAL_SPAM_LONG(1.00)[1.000]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; RCVD_COUNT_TWO(0.00)[2]; MAILMAN_DEST(0.00)[svn-src-all,svn-src-head]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jan 2021 20:45:05 -0000 On Tue, Aug 8, 2017 at 3:36 PM Alexander Motin 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.