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=-2.0 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 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 A9971C433E0 for ; Thu, 4 Jun 2020 15:34:46 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7F2C3207DF for ; Thu, 4 Jun 2020 15:34:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="VP8uwZiA"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="vZt0DMCf" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7F2C3207DF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=KimCM+7cMS8z6j72yfES0uA5wbc1DMlApVQF0g0/sJQ=; b=VP8uwZiAnem2xavLwb3fk8ELf RonKNrhU5bzSG5WCuWsRv0r1/97RGrhCtLyoEUYrPg0jPVG8zbd0nPw+zKc7rkcvVv6KVG7NBPPjK CAdCwoQTO/YBo51KQVZxB5XwyD1cBHTQEVB05/hrVINXZVU1Gwy4Y8BvKREn1JNmQTIx5ocyb65ng rg5wwCUXsGnlJzNaryrf5dh8ysb0ep70iFdthvf9R2EwfmI6dYm0pJhugtJD9nP7kkC8rdfqWMdGz PoGUjrt6lwJ+BPbh0PqTfC1rxnAtLEAUgvjf3v/XvOF70Gq85Rzj1NsTyBi12XGSiYDpmD/kkqLNW q58bCcPeA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jgrtG-0008V8-5n; Thu, 04 Jun 2020 15:34:46 +0000 Received: from mail-wr1-x444.google.com ([2a00:1450:4864:20::444]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jgrtC-0008UF-SI for linux-arm-kernel@lists.infradead.org; Thu, 04 Jun 2020 15:34:44 +0000 Received: by mail-wr1-x444.google.com with SMTP id y17so6584901wrn.11 for ; Thu, 04 Jun 2020 08:34:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=D3By0h98qnolfLM0+QRpl2dqCpgdvgnNY+OucjDnKDY=; b=vZt0DMCfoJUEXzeTrBICKPyqGfuBe7vx1mxfTRfe7AMZfEO9MgZiR1mPQATjGg7sos L2gEppsjZRToGPjruQDj6PgcZ9k1LIUGD0iWIyOtnINMSac6a+7rCj3z4QC4lrZCdKFE k2yM8OXoWQeV2IMPxu2sJJA6xUKeMYmYvndml0HnOewX3ksq8F1r9goi9s79mQpwsSIb c5mAXaNTKzUmdLKel6x7irCbEI4DTv2oXtv/qN7Oxn1zzp0W/r8QazRbJWdm+blAf/6N mqCzGRjWNAgSpkjxibd5bm0M9tIZAW+bPd5JKLWcPAMtTD2J+9FPsJ68L+Qr8Bw98u2/ uiog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=D3By0h98qnolfLM0+QRpl2dqCpgdvgnNY+OucjDnKDY=; b=cAzSC4BX6xTBQa9TUj/tOwcYrAJJqI9ml1IGVgemy9NCt5z6NE2Lc3sl669ZynOR8r 2c6lAWlVvl+s6CjBSn+WuGWz00hQXO+4HFgCwIuJP3U8hNcSWEXFn1rTKsnuxemt/QV5 ZSFbCF/J1PdIvT0Gh/d8TG5MuVWT3CvV2O46ymscIic3LZIOAf/4gndiuUbBvIr+Fxjt tN5t7ZezdDk4wUFCAShlAuKIy0zCPyc1CuyByNGKFgFPkArNoS5lqZRmfGb/SSZDwRk8 C9tngdDPUEaoVmUBAWRAXB3FitYwaSm6kS22+QI2HGRLRnRkhsqTz/PKN0LEXU4Pb6fL LJGg== X-Gm-Message-State: AOAM532jNTgjo13RImPyP7/cVsJiqeoidGp6ALwdVvg5OBdFMogHj6+f qVlIbtG663BDrOxfLxTlBCy2588+kY4= X-Google-Smtp-Source: ABdhPJxfqtpJqbRTF++Qa4Ke2KxsNtWe0xpkNWVokflegjt7JKKLmL/tz6wJQHjKWgmRMZK5uAnt/Q== X-Received: by 2002:adf:f0d2:: with SMTP id x18mr4926753wro.250.1591284880431; Thu, 04 Jun 2020 08:34:40 -0700 (PDT) Received: from [192.168.8.102] ([194.230.155.251]) by smtp.gmail.com with ESMTPSA id q1sm7431317wmc.12.2020.06.04.08.34.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 04 Jun 2020 08:34:39 -0700 (PDT) Subject: Re: [PATCH v3 2/7] documentation for stats_fs To: Randy Dunlap , Emanuele Giuseppe Esposito , kvm@vger.kernel.org References: <20200526110318.69006-1-eesposit@redhat.com> <20200526110318.69006-3-eesposit@redhat.com> From: Emanuele Giuseppe Esposito Message-ID: Date: Thu, 4 Jun 2020 17:34:37 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200604_083442_916225_75B0F66F X-CRM114-Status: GOOD ( 22.33 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-s390@vger.kernel.org, linux-doc@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, Jonathan Adams , Christian Borntraeger , Alexander Viro , David Rientjes , linux-fsdevel@vger.kernel.org, Paolo Bonzini , linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, Jim Mattson Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, >> + >> +The STATS_FS_HIDDEN attribute won't affect the aggregation, it will only >> +block the creation of the files. > > Why does HIDDEN block the creation of files? instead of their visibility? The file itself is used to allow the user to view the content of a value. In order to make it hidden, the framework just doesn't create the file. The structure is still present and considered in statsfs, however. Hidden in this case means not visible at all thus not created, not the hidden file concept of dotted files (".filename") > >> + >> +Add values to parent and child (also here order doesn't matter):: >> + >> + struct kvm *base_ptr = kmalloc(..., sizeof(struct kvm)); >> + ... >> + stats_fs_source_add_values(child_source, kvm_stats, base_ptr, 0); >> + stats_fs_source_add_values(parent_source, kvm_stats, NULL, STATS_FS_HIDDEN); >> + >> +``child_source`` will be a simple value, since it has a non-NULL base >> +pointer, while ``parent_source`` will be an aggregate. During the adding >> +phase, also values can optionally be marked as hidden, so that the folder >> +and other values can be still shown. >> + >> +Of course the same ``struct stats_fs_value`` array can be also passed with a >> +different base pointer, to represent the same value but in another instance >> +of the kvm struct. >> + >> +Search: >> + >> +Fetch a value from the child source, returning the value >> +pointed by ``(uint64_t *) base_ptr + kvm_stats[0].offset``:: >> + >> + uint64_t ret_child, ret_parent; >> + >> + stats_fs_source_get_value(child_source, &kvm_stats[0], &ret_child); >> + >> +Fetch an aggregate value, searching all subsources of ``parent_source`` for >> +the specified ``struct stats_fs_value``:: >> + >> + stats_fs_source_get_value(parent_source, &kvm_stats[0], &ret_parent); >> + >> + assert(ret_child == ret_parent); // check expected result >> + >> +To make it more interesting, add another child:: >> + >> + struct stats_fs_source child_source2 = stats_fs_source_create(0, "child2"); >> + >> + stats_fs_source_add_subordinate(parent_source, child_source2); >> + // now the structure is parent -> child1 >> + // -> child2 > > Is that the same as parent -> child1 -> child2 > ? It could almost be read as > parent -> child1 > parent -> child2 No the example in the documentation shows the relationship parent -> child1 and parent -> child2. It's not the same as parent -> child1 -> child2. In order to do the latter, one would need to do: stats_fs_source_add_subordinate(parent_source, child_source1); stats_fs_source_add_subordinate(child_source1, child_source2); Hope that this clarifies it. > > Whichever it is, can you make it more explicit, please? > > >> + >> + struct kvm *other_base_ptr = kmalloc(..., sizeof(struct kvm)); >> + ... >> + stats_fs_source_add_values(child_source2, kvm_stats, other_base_ptr, 0); >> + >> +Note that other_base_ptr points to another instance of kvm, so the struct >> +stats_fs_value is the same but the address at which they point is not. >> + >> +Now get the aggregate value:: >> + >> + uint64_t ret_child, ret_child2, ret_parent; >> + >> + stats_fs_source_get_value(child_source, &kvm_stats[0], &ret_child); >> + stats_fs_source_get_value(parent_source, &kvm_stats[0], &ret_parent); >> + stats_fs_source_get_value(child_source2, &kvm_stats[0], &ret_child2); >> + >> + assert((ret_child + ret_child2) == ret_parent); >> + >> +Cleanup:: >> + >> + stats_fs_source_remove_subordinate(parent_source, child_source); >> + stats_fs_source_revoke(child_source); >> + stats_fs_source_put(child_source); >> + >> + stats_fs_source_remove_subordinate(parent_source, child_source2); >> + stats_fs_source_revoke(child_source2); >> + stats_fs_source_put(child_source2); >> + >> + stats_fs_source_put(parent_source); >> + kfree(other_base_ptr); >> + kfree(base_ptr); >> + >> +Calling stats_fs_source_revoke is very important, because it will ensure > > stats_fs_source_revoke() > >> +that stats_fs will not access the data that were passed to >> +stats_fs_source_add_value for this source. >> + >> +Because open files increase the reference count for a stats_fs_source, the >> +source can end up living longer than the data that provides the values for >> +the source. Calling stats_fs_source_revoke just before the backing data > > stats_fs_source_revoke() > >> +is freed avoids accesses to freed data structures. The sources will return >> +0. >> + >> +This is not needed for the parent_source, since it just contains >> +aggregates that would be 0 anyways if no matching child value exist. >> + >> +API Documentation >> +================= >> + >> +.. kernel-doc:: include/linux/stats_fs.h >> + :export: fs/stats_fs/*.c >> \ No newline at end of file > > Please fix that. ^^^^^ > > > Thanks for the documentation. > Thank you for the feedback, Emanuele _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel