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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 341FAC433FE for ; Thu, 6 Oct 2022 21:07:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8616A6B0072; Thu, 6 Oct 2022 17:07:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 811046B0073; Thu, 6 Oct 2022 17:07:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6D8496B0074; Thu, 6 Oct 2022 17:07:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5949A6B0072 for ; Thu, 6 Oct 2022 17:07:08 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 25F17A018B for ; Thu, 6 Oct 2022 21:07:08 +0000 (UTC) X-FDA: 79991759736.25.3B3CBC1 Received: from mail-pg1-f173.google.com (mail-pg1-f173.google.com [209.85.215.173]) by imf17.hostedemail.com (Postfix) with ESMTP id 4817C40013 for ; Thu, 6 Oct 2022 21:07:06 +0000 (UTC) Received: by mail-pg1-f173.google.com with SMTP id r18so2878532pgr.12 for ; Thu, 06 Oct 2022 14:07:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date; bh=5bTBLyGPfmCxkvCIzfZznH2Ifhb8gJd3z2V25tJPiD0=; b=dKk8bfqpece8TZo84O+skqcxluLPH+y9y7rV2xLWxGfrjAgtytR3zgHSpZ2ynt5dd4 HrXCLreff/N1dVyFdxU1ljT97CZnmWHCQvZTLZLzkYIdeTF3nPznmMAv2eqk/v89e/QI YSCiqi5GugQ3S4/HoAYp6bqCN8QBAuK3gvvHCvwqwJNuqUTBt7Qx0xMFlC/o5QwuwAeq 9tzB8Up2piU9iIE1Fm5H68ZtWkt9rU8a3E8nOtRZ322avw10TkgM532X0/Xv9wXjoYxZ gvP3ZwVOVGc2C+DydjHAcAKUpMkG9qc+gAQce2h3qJFc8STO4ZtM+Fy6jM8c4rW+1o76 C9dA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date; bh=5bTBLyGPfmCxkvCIzfZznH2Ifhb8gJd3z2V25tJPiD0=; b=qbTZ/olFmPPvlV1dPdOJMHjadzLW54FP/gu7l1iGPUEOXqWh5S0aMnn/ZKhBznm07J kl/lzbTTHjqhv97ngyqkw7hA/fGIPPpt62fCaGwBG6tfK33G6A8GLYpn5tUPJyxuZ31J r4dBXxMqZHQGkvg+iZ9j8bym882A1gTrEDQ2LpZmyuvSqNUK/HS7EP02NbAM12Rsgn2P rnndd3skk0CDWXMDVf6HWWVuGbiv5/V7ORgK164gaxtxzlo9LysR8julvGur5CyYKo13 jCPjN3fFkTmBwVcnkGQ+uNm8CSMX7x8QvSdKYGxI5Fg9qRTeLTSDCcqYIYcBnZDrvZ+7 jt5w== X-Gm-Message-State: ACrzQf17i8Hytm/CDr9A+Jx/0QyWP26GhG1tiVXe3OerwoToEVZy4fE6 UvyqK4kVoUojskZCKyzyTiA= X-Google-Smtp-Source: AMsMyM6P/PidTZjuJ1+qQsyjd+J50nx98115Wq9dUPuvR3L5nFtZc04pmmeSWfJ29n8vVG5DiIoMeA== X-Received: by 2002:a63:fa4f:0:b0:438:e26d:5ec with SMTP id g15-20020a63fa4f000000b00438e26d05ecmr1510069pgk.361.1665090424880; Thu, 06 Oct 2022 14:07:04 -0700 (PDT) Received: from smtpclient.apple (c-24-6-216-183.hsd1.ca.comcast.net. [24.6.216.183]) by smtp.gmail.com with ESMTPSA id n1-20020a17090a2bc100b0020a0571b354sm134439pje.57.2022.10.06.14.07.03 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 06 Oct 2022 14:07:04 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) Subject: Re: [PATCH v4 2/7] Enable balloon drivers to report inflated memory From: Nadav Amit In-Reply-To: Date: Thu, 6 Oct 2022 14:07:02 -0700 Cc: "Michael S. Tsirkin" , David Hildenbrand , Andrew Morton , kernel@openvz.org, Linux Virtualization , LKML , linux-mm@kvack.org Content-Transfer-Encoding: quoted-printable Message-Id: <42C75E59-696B-41D5-BD77-68EFF0B075C6@gmail.com> References: <20221005090158.2801592-1-alexander.atanasov@virtuozzo.com> <20221005090158.2801592-3-alexander.atanasov@virtuozzo.com> <88EDC41D-408F-4ADF-A933-0A6F36E5F262@gmail.com> To: Alexander Atanasov X-Mailer: Apple Mail (2.3696.120.41.1.1) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1665090426; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=5bTBLyGPfmCxkvCIzfZznH2Ifhb8gJd3z2V25tJPiD0=; b=tH6t3Ifxiok8xAimZQ7v9hUjOlsO3Z9kB36GHcJbQfH0ZSWZ2qu1ksZKONWuh8xCRs7JpB sYCzrEIzruwe15u1obxCJ6i5cAv22LFgQjG6p6w/+sqdhYK2/kByWHs/ucWrPi+N4ff4MG Ze63i4Lwu9x0tOiiCjRCx/qG/t1A3rA= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=dKk8bfqp; spf=pass (imf17.hostedemail.com: domain of nadav.amit@gmail.com designates 209.85.215.173 as permitted sender) smtp.mailfrom=nadav.amit@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1665090426; a=rsa-sha256; cv=none; b=N94ttvtDNIKmnaEsuVlraqF+LUMbQk3153jbe97bXfk1aNpOIu6BNvxrylPbZMhBkZlaDG xCRCGOMqBD/PS1ecRBiQZYXaJV9301AMy4/DiP5RAn0jvnzHOpRf/5M3hfUIq1PObx9+RC 7wVWnY1hH8VkjCfdAxuM0jnwahd2QEo= X-Rspam-User: Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=dKk8bfqp; spf=pass (imf17.hostedemail.com: domain of nadav.amit@gmail.com designates 209.85.215.173 as permitted sender) smtp.mailfrom=nadav.amit@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Stat-Signature: mybrksf8sz7bmnuf93xeerrptq9skcjc X-Rspamd-Queue-Id: 4817C40013 X-Rspamd-Server: rspam02 X-HE-Tag: 1665090426-185335 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: On Oct 6, 2022, at 12:34 AM, Alexander Atanasov = wrote: > Hello, >=20 >=20 > On 5.10.22 20:25, Nadav Amit wrote: >> On Oct 5, 2022, at 2:01 AM, Alexander Atanasov = wrote: >>> Add counters to be updated by the balloon drivers. >>> Create balloon notifier to propagate changes. >> I missed the other patches before (including this one). Sorry, but = next >> time, please cc me. >=20 > You are CCed in the cover letter since the version. I will add CC to = you > in the individual patches if you want so. Thanks. Just to clarify - I am not attacking you. It=E2=80=99s more of me making = an excuse for not addressing some issues in earlier versions. >> I was looking through the series and I did not see actual users of = the >> notifier. Usually, it is not great to build an API without users. >=20 >=20 > You are right. I hope to get some feedback/interest from potential = users that i mentioned in the cover letter. I will probably split the = notifier > in separate series. To make it usefull it will require more changes. > See bellow more about them. Fair, but this is something that is more suitable for RFC. Otherwise, = more likely than not - your patches would go in as is. >> [snip] >>> + >>> +static int balloon_notify(unsigned long val) >>> +{ >>> + return srcu_notifier_call_chain(&balloon_chain, val, NULL); >> Since you know the inflated_kb value here, why not to use it as an = argument >> to the callback? I think casting to (void *) and back is best. But = you can >> also provide pointer to the value. Doesn=E2=80=99t it sound better = than having >> potentially different notifiers reading different values? >=20 > My current idea is to have a struct with current and previous value, > may be change in percents. The actual value does not matter to anyone > but the size of change does. When a user gets notified it can act upon > the change - if it is small it can ignore it , if it is above some = threshold it can act - if it makes sense for some receiver is can = accumulate changes from several notification. Other option/addition is = to have si_meminfo_current(..) and totalram_pages_current(..) that = return values adjusted with the balloon values. >=20 > Going further - there are few places that calculate something based on = available memory that do not have sysfs/proc interface for setting = limits. Most of them work in percents so they can be converted to do = calculations when they get notification. >=20 > The one that have interface for configuration but use memory values = can be handled in two ways - convert to use percents of what is = available or extend the notifier to notify userspace which in turn to do = calculations and update configuration. I really need to see code to fully understand what you have in mind. Division, as you know, is not something that we really want to do very frequently. >> Anyhow, without users (actual notifiers) it=E2=80=99s kind of hard to = know how >> reasonable it all is. For instance, is it balloon_notify() supposed = to >> prevent further balloon inflating/deflating until the notifier = completes? >=20 > No, we must avoid that at any cost. >=20 >> Accordingly, are callers to balloon_notify() expected to relinquish = locks >> before calling balloon_notify() to prevent deadlocks and high = latency? >=20 > My goal is to avoid any possible impact on performance. Drivers are = free to delay notifications if they get in the way. (I see that i need = to move the notification after the semaphore in the vmw driver - i = missed that - will fix in the next iterration.) > Deadlocks - depends on the users but a few to none will possibly have = to deal with common locks. I will need to see the next version to give better feedback. One more = thing that comes to mind though is whether saving the balloon size in multiple places (both mem_balloon_inflated_total_kb and each balloon=E2=80=99s = accounting) is the right way. It does not sounds very clean. Two other options is to move *all* the accounting to your new mem_balloon_inflated_total_kb-like interface or expose some per-balloon function to get the balloon size (indirect-function-call would likely = have some overhead though). Anyhow, I am not crazy about having the same data replicated. Even from reading the code stand-of-view it is not intuitive.