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=-3.8 required=3.0 tests=BAYES_00, 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 966BBC388F9 for ; Fri, 20 Nov 2020 02:34:28 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EAFE12158C for ; Fri, 20 Nov 2020 02:34:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EAFE12158C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 0097C6B0036; Thu, 19 Nov 2020 21:34:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id EFC3F6B005C; Thu, 19 Nov 2020 21:34:26 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E12866B005D; Thu, 19 Nov 2020 21:34:26 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0158.hostedemail.com [216.40.44.158]) by kanga.kvack.org (Postfix) with ESMTP id AA7586B0036 for ; Thu, 19 Nov 2020 21:34:26 -0500 (EST) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 41A143647 for ; Fri, 20 Nov 2020 02:34:26 +0000 (UTC) X-FDA: 77503227732.05.fear54_5f156e727347 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id 2517A1807B30D for ; Fri, 20 Nov 2020 02:34:26 +0000 (UTC) X-HE-Tag: fear54_5f156e727347 X-Filterd-Recvd-Size: 9082 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by imf25.hostedemail.com (Postfix) with ESMTP for ; Fri, 20 Nov 2020 02:34:23 +0000 (UTC) IronPort-SDR: S9aGQrRouTdcxQf28XUHwLkzbi2Ac3SHJR98Z0T0QqHDOV3HvJnT5jxmBypDvFBrvnL1391pyV 4nuL2NclL4fQ== X-IronPort-AV: E=McAfee;i="6000,8403,9810"; a="168829335" X-IronPort-AV: E=Sophos;i="5.78,354,1599548400"; d="scan'208";a="168829335" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Nov 2020 18:34:22 -0800 IronPort-SDR: pyr0PZg1wIIwvBczesvoSnPc0oCSt5HzcldCuTNhmi331eXLMOSXTJDPySoUzifOGPxrcoSXoQ Zz4fwbDT9WuQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.78,354,1599548400"; d="scan'208";a="535008625" Received: from yhuang-dev.sh.intel.com (HELO yhuang-dev) ([10.239.159.50]) by fmsmga005.fm.intel.com with ESMTP; 19 Nov 2020 18:34:20 -0800 From: "Huang\, Ying" To: Uladzislau Rezki Cc: huang ying , Andrew Morton , linux-mm@kvack.org, LKML , Hillf Danton , Michal Hocko , Matthew Wilcox , Oleksiy Avramchenko , Steven Rostedt , Christoph Hellwig Subject: Re: [PATCH 2/2] mm/vmalloc: rework the drain logic References: <20201116220033.1837-1-urezki@gmail.com> <20201116220033.1837-2-urezki@gmail.com> <20201117130434.GA10769@pc636> <20201118161623.GA21171@pc636> <87mtzeunsi.fsf@yhuang-dev.intel.com> <20201119173604.GA991@pc636> Date: Fri, 20 Nov 2020 10:34:19 +0800 In-Reply-To: <20201119173604.GA991@pc636> (Uladzislau Rezki's message of "Thu, 19 Nov 2020 18:36:04 +0100") Message-ID: <87zh3cu578.fsf@yhuang-dev.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Uladzislau Rezki writes: > On Thu, Nov 19, 2020 at 09:40:29AM +0800, Huang, Ying wrote: >> Uladzislau Rezki writes: >> >> > On Wed, Nov 18, 2020 at 10:44:13AM +0800, huang ying wrote: >> >> On Tue, Nov 17, 2020 at 9:04 PM Uladzislau Rezki wrote: >> >> > >> >> > On Tue, Nov 17, 2020 at 10:37:34AM +0800, huang ying wrote: >> >> > > On Tue, Nov 17, 2020 at 6:00 AM Uladzislau Rezki (Sony) >> >> > > wrote: >> >> > > > >> >> > > > A current "lazy drain" model suffers from at least two issues. >> >> > > > >> >> > > > First one is related to the unsorted list of vmap areas, thus >> >> > > > in order to identify the [min:max] range of areas to be drained, >> >> > > > it requires a full list scan. What is a time consuming if the >> >> > > > list is too long. >> >> > > > >> >> > > > Second one and as a next step is about merging all fragments >> >> > > > with a free space. What is also a time consuming because it >> >> > > > has to iterate over entire list which holds outstanding lazy >> >> > > > areas. >> >> > > > >> >> > > > See below the "preemptirqsoff" tracer that illustrates a high >> >> > > > latency. It is ~24 676us. Our workloads like audio and video >> >> > > > are effected by such long latency: >> >> > > >> >> > > This seems like a real problem. But I found there's long latency >> >> > > avoidance mechanism in the loop in __purge_vmap_area_lazy() as >> >> > > follows, >> >> > > >> >> > > if (atomic_long_read(&vmap_lazy_nr) < resched_threshold) >> >> > > cond_resched_lock(&free_vmap_area_lock); >> >> > > >> >> > I have added that "resched threshold" because of on my tests i could >> >> > simply hit out of memory, due to the fact that a drain work is not up >> >> > to speed to process such long outstanding list of vmap areas. >> >> >> >> OK. Now I think I understand the problem. For free area purging, >> >> there are multiple "producers" but one "consumer", and it lacks enough >> >> mechanism to slow down the "producers" if "consumer" can not catch up. >> >> And your patch tries to resolve the problem via accelerating the >> >> "consumer". >> >> >> > Seems, correct. But just in case one more time: >> > >> > the cond_resched_lock was added once upon a time to get rid of long >> > preemption off time. Due to dropping the lock, "producers" can start >> > generate further vmap area, so "consumer" can not catch up. Seems >> >> Yes. And in theory there are vfree() storm, that is, thousands vfree() >> can be called in short time. But I don't think that's practical use >> case. >> >> > Later on, a resched threshold was added. It is just a simple protection >> > threshold, passing which, a freeing is prioritized back over allocation, >> > so we guarantee that we do not hit out of memory. >> >> Yes. That can accelerate freeing if necessary. >> >> >> >> >> That isn't perfect, but I think we may have quite some opportunities >> >> to merge the free areas, so it should just work. >> >> >> > Yes, merging opportunity should do the work. But of course there are >> > exceptions. >> > >> >> And I found the long latency avoidance logic in >> >> __purge_vmap_area_lazy() appears problematic, >> >> >> >> if (atomic_long_read(&vmap_lazy_nr) < resched_threshold) >> >> cond_resched_lock(&free_vmap_area_lock); >> >> >> >> Shouldn't it be something as follows? >> >> >> >> if (i >= BATCH && atomic_long_read(&vmap_lazy_nr) < >> >> resched_threshold) { >> >> cond_resched_lock(&free_vmap_area_lock); >> >> i = 0; >> >> } else >> >> i++; >> >> >> >> This will accelerate the purging via batching and slow down vmalloc() >> >> via holding free_vmap_area_lock. If it makes sense, can we try this? >> >> >> > Probably we can switch to just using "batch" methodology: >> > >> > >> > if (!(i++ % batch_threshold)) >> > cond_resched_lock(&free_vmap_area_lock); >> > >> >> That's the typical long latency avoidance method. >> >> > The question is, which value we should use as a batch_threshold: 100, 1000, etc. >> >> I think we can do some measurement to determine it? >> > Hmm.. looking at it one more time i do not see what batching solves. Without batch protection, we may release the lock and CPU anytime during looping if "vmap_lazy_nr < resched_threshold". Too many vmalloc/vfree may be done during that. So I think we can restrict it. Batching can improve the performance of purging itself too. > Anyway we need to have some threshold(what we do have), that regulates > a priority between vmalloc()/vfree(). > > What we can do more with it are: > > - purging should be just performed asynchronously in workqueue context. > Giving the fact, that now we also do a merge of outstanding areas, the > data structure(rb-tree) will not be so fragmented. Async works only if there are idle CPU time on other CPUs. And it may punish other innocent workloads instead of the heavy vmalloc/vfree users. So we should be careful about that. > - lazy_max_pages() can slightly be decreased. If there are existing > workloads which suffer from such long value. It would be good to get > real complains and evidence. > >> > Apart of it and in regard to CONFIG_KASAN_VMALLOC, it seems that we are not >> > allowed to drop the free_vmap_area_lock at all. Because any simultaneous >> > allocations are not allowed within a drain region, so it should occur in >> > disjoint regions. But i need to double check it. >> > >> >> >> >> And, can we reduce lazy_max_pages() to control the length of the >> >> purging list? It could be > 8K if the vmalloc/vfree size is small. >> >> >> > We can adjust it for sure. But it will influence on number of global >> > TLB flushes that must be performed. >> >> Em... For example, if we set it to 100, then the number of the TLB >> flushes can be reduced to 1% of the un-optimized implementation >> already. Do you think so? >> > If we set lazy_max_pages() to vague value such as 100, the performance > will be just destroyed. Sorry, my original words weren't clear enough. What I really want to suggest is to control the length of the purging list instead of reduce lazy_max_pages() directly. That is, we can have a "atomic_t nr_purge_item" to record the length of the purging list and start purging if (vmap_lazy_nr > lazy_max_pages && nr_purge_item > max_purge_item). vmap_lazy_nr is to control the virtual address space, nr_purge_item is to control the batching purging latency. "100" is just an example, the real value should be determined according to the test results. Best Regards, Huang, Ying