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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0319CA0EC3 for ; Mon, 11 Sep 2023 22:09:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357560AbjIKWFo (ORCPT ); Mon, 11 Sep 2023 18:05:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38274 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244419AbjIKU2q (ORCPT ); Mon, 11 Sep 2023 16:28:46 -0400 Received: from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com [IPv6:2a00:1450:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0FD68185 for ; Mon, 11 Sep 2023 13:28:41 -0700 (PDT) Received: by mail-ej1-x62c.google.com with SMTP id a640c23a62f3a-9a9d6b98845so1165773766b.0 for ; Mon, 11 Sep 2023 13:28:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1694464119; x=1695068919; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=u1FlQJT5CLEqBN4p3+flYnqVBEBTd/hK6rhM4bfpcQo=; b=keSkKekvqwIC2nMJHzCWV/AeFNM2WTD6VoQF78ASEJw6a0vvmfjndXz1+xjkuOChsS AEPn+EtmPW+NSRhWPsfuQfZ42z9h9WtWX5zeJNVeWbDc/c8zrl+qrcDHh5gj7DJ65VeF 4dTbrH73PijNnjfybihpa0BetXAbRCrKkoOYEp61mSVu0HAxJTOvmEC7S1Czn3SxwPol cnAEn2bk5Ilzdz7l19zN5pMUvHJE2YnxpOzA3M40RM39fmvYKpkwBjDROLy2iuVsN7nf zM0Ll67J2fpZnmRUbSxOJJivPjTsFFLmC/AzHzLja/qzlo/unauRRG3aWaCKMOiQ0d/5 ZCOA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694464119; x=1695068919; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=u1FlQJT5CLEqBN4p3+flYnqVBEBTd/hK6rhM4bfpcQo=; b=kf8qqKilMeRAYwPCthpKT5veXqMGD22b4x77ZZecGLr6F/VZtNwXvKZme2uMKHYwE4 j2gFwbNGNQHoPqYMRUsGoEzXNYzjQEWN15uDR9yFztLXqz0CQ5SFn5lgozW1cI8tELGI OxQUc1FYEw9wiShonMwKn73Ubn5Hljol8vdyVvTCEkgJ7zR5cXDc8H2gwI2/XX94zMhK FQchrLg3AFcLSbwsYbuqnhDIi88njDxrdAk58R8biyutNHBHhVLCmns8hpIFsp81s40N E4HAecXFa9YIzmRh0sC9kKSPoBWYOUXAbViGN9fwfmuJ8h6HX+O+rcVRfnSSyx7pceCO +5/w== X-Gm-Message-State: AOJu0YxN9oD0NPa9j4LrBLv44Udv52FQfofPfpL7hwANny0OMIaqmJNo XbV5IIg+v3fOmjUn7TzL1xl4xLHWgHbYcZs8uXvyIQ== X-Google-Smtp-Source: AGHT+IEH+9F3gycVz0qnwgTvEbn9nbBIu3p9WJ/4/wKz5bCMECF/tNxMgywpY5TqLFcGd5V515Ju3Zc6mdoqTJJ/WNs= X-Received: by 2002:a17:907:7244:b0:9a9:f6e7:43fe with SMTP id ds4-20020a170907724400b009a9f6e743femr917666ejc.24.1694464119298; Mon, 11 Sep 2023 13:28:39 -0700 (PDT) MIME-Version: 1.0 References: <20230831165611.2610118-1-yosryahmed@google.com> <20230831165611.2610118-5-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Mon, 11 Sep 2023 13:28:00 -0700 Message-ID: Subject: Re: [PATCH v4 4/4] mm: memcg: use non-unified stats flushing for userspace reads To: Tejun Heo Cc: Wei Xu , Michal Hocko , Andrew Morton , Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , Ivan Babrou , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Waiman Long , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Greg Thelen Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Sep 11, 2023 at 1:21=E2=80=AFPM Tejun Heo wrote: > > Hello, > > On Mon, Sep 11, 2023 at 01:01:25PM -0700, Wei Xu wrote: > > Yes, it is the same test (10K contending readers). The kernel change > > is to remove stats_user_flush_mutex from mem_cgroup_user_flush_stats() > > so that the concurrent mem_cgroup_user_flush_stats() requests directly > > contend on cgroup_rstat_lock in cgroup_rstat_flush(). > > I don't think it'd be a good idea to twist rstat and other kernel interna= l > code to accommodate 10k parallel readers. If we want to support that, let= 's > explicitly support that by implementing better batching in the read path. > The only guarantee you need is that there has been at least one flush sin= ce > the read attempt started, so we can do sth like the following in the read > path: > > 1. Grab a waiter lock. Remember the current timestamp. > > 2. Try lock flush mutex. If obtained, drop the waiter lock, flush. Regrab > the waiter lock, update the latest flush time to my start time, wake u= p > waiters on the waitqueue (maybe do custom wakeups based on start time?= ). > > 3. Release the waiter lock and sleep on the waitqueue. > > 4. When woken up, regarb the waiter lock, compare whether the latest flus= h > timestamp is later than my start time, if so, return the latest result= . > If not go back to #2. > > Maybe the above isn't the best way to do it but you get the general idea. > When you have that many concurrent readers, most of them won't need to > actually flush. I am testing something vaguely similar to this conceptually, but doesn't depend on timestamps. I replaced the mutex with a semaphore, and I added a fallback logic to unified flushing with a timeout: static void mem_cgroup_user_flush_stats(struct mem_cgroup *memcg) { static DEFINE_SEMAPHORE(user_flush_sem, 1); if (atomic_read(&stats_flush_order) <=3D STATS_FLUSH_THRESHOLD) return; if (!down_timeout(&user_flush_sem, msecs_to_jiffies(1))) { do_stats_flush(memcg); up(&user_flush_sem); } else { do_unified_stats_flush(true); } } In do_unified_stats_flush(), I added a wait argument. If set, the caller will wait for any ongoing flushers before returning (but it never attempts to flush, so no contention on the underlying rstat lock). I implemented this using completions. I am running some tests now, but this should make sure userspace read latency is bounded by 1ms + unified flush time. We basically attempt to flush our subtree only, if we can't after 1ms, we fallback to unified flushing. Another benefit I am seeing here is that I tried switching in-kernel flushers to also use the completion in do_unified_stats_flush(). Basically instead of skipping entirely when someone else is flushing, they just wait for them to finish (without being serialized or contending the lock). I see no regressions in my parallel reclaim benchmark. This should make sure no one ever skips a flush, while still avoiding too much serialization/contention. I suspect this should make reclaim heuristics (and other in-kernel flushers) slightly better. I will run Wei's benchmark next to see how userspace read latency is affect= ed. > > Thanks. > > -- > tejun