From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67FB7C433ED for ; Wed, 7 Apr 2021 06:53:17 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9AAC761151 for ; Wed, 7 Apr 2021 06:53:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9AAC761151 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sina.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id CBCAD6B007D; Wed, 7 Apr 2021 02:53:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C21CC6B007E; Wed, 7 Apr 2021 02:53:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ABF3D6B0080; Wed, 7 Apr 2021 02:53:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0081.hostedemail.com [216.40.44.81]) by kanga.kvack.org (Postfix) with ESMTP id 89B326B007D for ; Wed, 7 Apr 2021 02:53:15 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 3F6648249980 for ; Wed, 7 Apr 2021 06:53:15 +0000 (UTC) X-FDA: 78004654350.19.1DE69D8 Received: from r3-11.sinamail.sina.com.cn (r3-11.sinamail.sina.com.cn [202.108.3.11]) by imf10.hostedemail.com (Postfix) with SMTP id 3EBD440002C2 for ; Wed, 7 Apr 2021 06:53:10 +0000 (UTC) Received: from unknown (HELO localhost.localdomain)([221.199.207.229]) by sina.com (172.16.97.27) with ESMTP id 606D56D200022AE4; Wed, 7 Apr 2021 14:53:09 +0800 (CST) X-Sender: hdanton@sina.com X-Auth-ID: hdanton@sina.com X-SMAIL-MID: 37798049283295 From: Hillf Danton To: Dan Schatzberg Cc: Hillf Danton , Jens Axboe , linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 1/3] loop: Use worker per cgroup instead of kworker Date: Wed, 7 Apr 2021 14:53:00 +0800 Message-Id: <20210407065300.1478-1-hdanton@sina.com> In-Reply-To: References: <20210402191638.3249835-1-schatzberg.dan@gmail.com> <20210403020902.1384-1-hdanton@sina.com> MIME-Version: 1.0 X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 3EBD440002C2 X-Stat-Signature: nyctr9bbt5grgrrbsgtdo37i5eys7gtn Received-SPF: none (sina.com>: No applicable sender policy available) receiver=imf10; identity=mailfrom; envelope-from=""; helo=r3-11.sinamail.sina.com.cn; client-ip=202.108.3.11 X-HE-DKIM-Result: none/none X-HE-Tag: 1617778390-868724 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 6 Apr 2021 Dan Schatzberg wrote: >On Sat, Apr 03, 2021 at 10:09:02AM +0800, Hillf Danton wrote: >> On Fri, 2 Apr 2021 12:16:32 Dan Schatzberg wrote: >> > +queue_work: >> > + if (worker) { >> > + /* >> > + * We need to remove from the idle list here while >> > + * holding the lock so that the idle timer doesn't >> > + * free the worker >> > + */ >> > + if (!list_empty(&worker->idle_list)) >> > + list_del_init(&worker->idle_list); >>=20 >> Nit, only queue work if the worker is inactive - otherwise it is takin= g >> care of the cmd_list. > >By worker is inactive, you mean worker is on the idle_list? Yes, I >think you're right that queue_work() is unnecessary in that case since >each worker checks empty cmd_list then adds itself to idle_list under >the lock. > >>=20 >> > + work =3D &worker->work; >> > + cmd_list =3D &worker->cmd_list; >> > + } else { >> > + work =3D &lo->rootcg_work; >> > + cmd_list =3D &lo->rootcg_cmd_list; >> > + } >> > + list_add_tail(&cmd->list_entry, cmd_list); >> > + queue_work(lo->workqueue, work); >> > + spin_unlock_irq(&lo->lo_work_lock); >> > } >> [...] >> > + /* >> > + * We only add to the idle list if there are no pending cmds >> > + * *and* the worker will not run again which ensures that it >> > + * is safe to free any worker on the idle list >> > + */ >> > + if (worker && !work_pending(&worker->work)) { >>=20 >> The empty cmd_list is a good enough reason for worker to become idle. > >This is only true with the above change to avoid a gratuitous >queue_work(), right? It is always true because of the empty cmd_list - the idle_list is the on= ly place for the worker to go at this point. >Otherwise we run the risk of freeing a worker >concurrently with loop_process_work() being invoked. My suggestion is a minor optimization at most without any change to remov= ing worker off the idle_list on queuing work - that cuts the risk for you.