Received: by 2002:a05:6a10:f347:0:0:0:0 with SMTP id d7csp10687591pxu; Wed, 30 Dec 2020 08:56:27 -0800 (PST) X-Google-Smtp-Source: ABdhPJwktm2TcQ384SXie8aT3QzEwVt8srIqAhH91i+1VFYL1FBFIyCc8Dcw3zN3CMeKHDem6K/f X-Received: by 2002:a17:906:c045:: with SMTP id bm5mr44028721ejb.190.1609347386856; Wed, 30 Dec 2020 08:56:26 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1609347386; cv=none; d=google.com; s=arc-20160816; b=v00fvXc729HwGSxFBl7QCTTh7pIKIZ61zGOr6Xq0uWSZ/kZueeJ1GO/wu7gcgWQTxE z6bahxH2J39JDKbOdG+lgLm2adAnKKutM1+mNnqCmEwl4xuH1qNYuhtkPaXW0AygZ60j lO8kwUClKwu0ucqqrEykQn0YjgWhtA7GpiTy7hxYBKnAh80hv5iwAajUQqE1MHQbRaoC y0oqFVRJB25bgVfbZMv0Liy1jXRT588jldn9s/7P8zs6t6nrfz2D5jDvfiucv54mE+NN MLeHVSMcxcDLLrzYUVFO5ytZMnSqbx0l6hfDi22l8EHuNxj1So+fDurlMRJpsVYMYoas kv7g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject; bh=jTxUoM12pT2emFeF9v3tc3fo2AaNbDhkT4uICo2t1NQ=; b=TNY+LAYZMFfCWg1mypq4Rn6JtMM6TyQhK8oD1iWb3OnaZsLm8Kd8B9fZ2dFh00ipvG zICT3IIgJKGWBkge3ATshW0+xVGIB9RefgFh30I7qwzIISA82O17qVsP7vbfIVG+HXNf LAcNuDU/7ePDKY79RyInqYGUkxg0VHTXl/1UM8oAnBUqgeDyAzS/xGU70XjK6jeZJHk2 zUHJZv02iazGCP8C9Jb/zcQ/Cnt0XH5Ic2gH2TfqTdoTy45qL58fSHWuDYSu+KdHervg QG38GxuGKA275Kf5XKauOvggHVjpnU9oFIiL/fizCbz7D94WJEGzwYOYZhvsxu61JlOz DPCg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id bu13si20581326ejb.588.2020.12.30.08.56.03; Wed, 30 Dec 2020 08:56:26 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726598AbgL3Qy5 (ORCPT + 99 others); Wed, 30 Dec 2020 11:54:57 -0500 Received: from mx3.molgen.mpg.de ([141.14.17.11]:34983 "EHLO mx1.molgen.mpg.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726214AbgL3Qy5 (ORCPT ); Wed, 30 Dec 2020 11:54:57 -0500 Received: from [192.168.0.8] (ip5f5aef2f.dynamic.kabel-deutschland.de [95.90.239.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: buczek) by mx.molgen.mpg.de (Postfix) with ESMTPSA id 285FC20646219; Wed, 30 Dec 2020 17:54:14 +0100 (CET) Subject: Re: [PATCH] xfs: Wake CIL push waiters more reliably To: Hillf Danton Cc: linux-xfs@vger.kernel.org, Brian Foster , Dave Chinner , LKML , it+linux-xfs@molgen.mpg.de References: <1705b481-16db-391e-48a8-a932d1f137e7@molgen.mpg.de> <20201230024642.2171-1-hdanton@sina.com> From: Donald Buczek Message-ID: Date: Wed, 30 Dec 2020 17:54:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201230024642.2171-1-hdanton@sina.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 30.12.20 03:46, Hillf Danton wrote: > On Wed, 30 Dec 2020 00:56:27 +0100 >> Threads, which committed items to the CIL, wait in the xc_push_wait >> waitqueue when used_space in the push context goes over a limit. These >> threads need to be woken when the CIL is pushed. >> >> The CIL push worker tries to avoid the overhead of calling wake_all() >> when there are no waiters waiting. It does so by checking the same >> condition which caused the waits to happen. This, however, is >> unreliable, because ctx->space_used can actually decrease when items are >> recommitted. If the value goes below the limit while some threads are >> already waiting but before the push worker gets to it, these threads are >> not woken. > > Looks like you are fixing a typo in c7f87f3984cf ("xfs: fix > use-after-free on CIL context on shutdown") in mainline, and > it may mean > > /* > * Wake up any background push waiters now this context is being pushed > * if we are no longer over the space limit > */ > > given waiters throttled for comsuming more space than limit in > xlog_cil_push_background(). I'm not sure, I understand you correctly. Do you suggest to update the comment to "...if we are no longer over the space limit" and change the code to `if (ctx->space_used < XLOG_CIL_BLOCKING_SPACE_LIMIT(log))` ? I don't think, that would be correct. The current push context is most probably still over the limit if it ever was. It is exceptional, that the few bytes, which might be returned to ctx->space_used, bring us back below the limit. The new context, on the other hand, will have space_used=0. We need to resume any thread which is waiting for the push. Best Donald >> Always wake all CIL push waiters. Test with waitqueue_active() as an >> optimization. This is possible, because we hold the xc_push_lock >> spinlock, which prevents additions to the waitqueue. >> >> Signed-off-by: Donald Buczek >> --- >> fs/xfs/xfs_log_cil.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c >> index b0ef071b3cb5..d620de8e217c 100644 >> --- a/fs/xfs/xfs_log_cil.c >> +++ b/fs/xfs/xfs_log_cil.c >> @@ -670,7 +670,7 @@ xlog_cil_push_work( >> /* >> * Wake up any background push waiters now this context is being pushed. >> */ >> - if (ctx->space_used >= XLOG_CIL_BLOCKING_SPACE_LIMIT(log)) >> + if (waitqueue_active(&cil->xc_push_wait)) >> wake_up_all(&cil->xc_push_wait); >> >> /* >> -- >> 2.26.2