From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A79329E47E for ; Tue, 31 Oct 2023 13:11:17 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 73CF91B0B1 for ; Tue, 31 Oct 2023 13:11:17 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 31 Oct 2023 13:11:16 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 02F7442DD5 for ; Tue, 31 Oct 2023 13:11:16 +0100 (CET) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Tue, 31 Oct 2023 13:10:59 +0100 Message-ID: <20231031121108.1130299-8-c.heiss@proxmox.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231031121108.1130299-1-c.heiss@proxmox.com> References: <20231031121108.1130299-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.015 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pve-devel] [PATCH installer v3 7/8] fix #4829: tui: setup: add new ZFS `arc_max` option X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Oct 2023 12:11:17 -0000 Signed-off-by: Christoph Heiss --- Changes v1 -> v2: * updated comment for ZfsBootdiskOptions::defaults_from() accordingly Changes v2 -> v2: * documented the unit of the return value of default_zfs_arc_max() proxmox-tui-installer/src/options.rs | 57 +++++++++++++++++++-- proxmox-tui-installer/src/setup.rs | 2 + proxmox-tui-installer/src/views/bootdisk.rs | 31 ++++++----- 3 files changed, 71 insertions(+), 19 deletions(-) diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs index 85b39b8..c9b036d 100644 --- a/proxmox-tui-installer/src/options.rs +++ b/proxmox-tui-installer/src/options.rs @@ -1,7 +1,9 @@ use std::net::{IpAddr, Ipv4Addr}; use std::{cmp, fmt}; -use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo}; +use crate::setup::{ + LocaleInfo, NetworkInfo, ProductConfig, ProxmoxProduct, RuntimeInfo, SetupInfo, +}; use crate::utils::{CidrAddress, Fqdn}; use crate::SummaryOption; @@ -190,21 +192,23 @@ pub struct ZfsBootdiskOptions { pub compress: ZfsCompressOption, pub checksum: ZfsChecksumOption, pub copies: usize, + pub arc_max: usize, pub disk_size: f64, pub selected_disks: Vec, } impl ZfsBootdiskOptions { - /// This panics if the provided slice is empty. - pub fn defaults_from(disks: &[Disk]) -> Self { - let disk = &disks[0]; + /// Panics if the disk list is empty. + pub fn defaults_from(runinfo: &RuntimeInfo, product_conf: &ProductConfig) -> Self { + let disk = &runinfo.disks[0]; Self { ashift: 12, compress: ZfsCompressOption::default(), checksum: ZfsChecksumOption::default(), copies: 1, + arc_max: default_zfs_arc_max(product_conf.product, runinfo.total_memory), disk_size: disk.size, - selected_disks: (0..disks.len()).collect(), + selected_disks: (0..runinfo.disks.len()).collect(), } } } @@ -444,6 +448,27 @@ impl InstallerOptions { } } +/// Calculates the default upper limit for the ZFS ARC size. +/// See also and +/// https://openzfs.github.io/openzfs-docs/Performance%20and%20Tuning/Module%20Parameters.html#zfs-arc-max +/// +/// # Arguments +/// * `product` - The product to be installed +/// * `total_memory` - Total memory installed in the system, in MiB +/// +/// # Returns +/// The default ZFS maximum ARC size in MiB for this system. +fn default_zfs_arc_max(product: ProxmoxProduct, total_memory: usize) -> usize { + if product != ProxmoxProduct::PVE { + // Use ZFS default for non-PVE + 0 + } else { + ((total_memory as f64) / 10.) + .round() + .clamp(64., 16. * 1024.) as usize + } +} + #[cfg(test)] mod tests { use super::*; @@ -470,6 +495,28 @@ mod tests { } } + #[test] + fn zfs_arc_limit() { + const TESTS: &[(usize, usize)] = &[ + (16, 64), // at least 64 MiB + (1024, 102), + (4 * 1024, 410), + (8 * 1024, 819), + (150 * 1024, 15360), + (160 * 1024, 16384), + (1024 * 1024, 16384), // maximum of 16 GiB + ]; + + for (total_memory, expected) in TESTS { + assert_eq!( + default_zfs_arc_max(ProxmoxProduct::PVE, *total_memory), + *expected + ); + assert_eq!(default_zfs_arc_max(ProxmoxProduct::PBS, *total_memory), 0); + assert_eq!(default_zfs_arc_max(ProxmoxProduct::PMG, *total_memory), 0); + } + } + #[test] fn network_options_from_setup_network_info() { let setup = dummy_setup_info(); diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs index 5575759..e9fe70d 100644 --- a/proxmox-tui-installer/src/setup.rs +++ b/proxmox-tui-installer/src/setup.rs @@ -114,6 +114,7 @@ struct InstallZfsOption { #[serde(serialize_with = "serialize_as_display")] checksum: ZfsChecksumOption, copies: usize, + arc_max: usize, } impl From for InstallZfsOption { @@ -123,6 +124,7 @@ impl From for InstallZfsOption { compress: opts.compress, checksum: opts.checksum, copies: opts.copies, + arc_max: opts.arc_max, } } } diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index 3addd6c..07ca5b7 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -16,7 +16,7 @@ use crate::{ FsType, LvmBootdiskOptions, ZfsBootdiskOptions, ZfsRaidLevel, FS_TYPES, ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS, }, - setup::{BootType, ProductConfig}, + setup::{BootType, ProductConfig, RuntimeInfo}, }; use crate::{setup::ProxmoxProduct, InstallerState}; @@ -123,10 +123,7 @@ impl AdvancedBootdiskOptionsView { .position(|t| *t == options.fstype) .unwrap_or_default(), ) - .on_submit({ - let disks = disks.to_owned(); - move |siv, fstype| Self::fstype_on_submit(siv, &disks, fstype) - }); + .on_submit(move |siv, fstype| Self::fstype_on_submit(siv, fstype)); let mut view = LinearLayout::vertical() .child(DummyView.full_width()) @@ -148,8 +145,9 @@ impl AdvancedBootdiskOptionsView { Self { view } } - fn fstype_on_submit(siv: &mut Cursive, disks: &[Disk], fstype: &FsType) { + fn fstype_on_submit(siv: &mut Cursive, fstype: &FsType) { let state = siv.user_data::().unwrap(); + let runinfo = state.runtime_info.clone(); let product_conf = state.setup_info.config.clone(); siv.call_on_name("advanced-bootdisk-options-dialog", |view: &mut Dialog| { @@ -159,13 +157,14 @@ impl AdvancedBootdiskOptionsView { view.remove_child(3); match fstype { FsType::Ext4 | FsType::Xfs => view.add_child( - LvmBootdiskOptionsView::new_with_defaults(&disks[0], &product_conf), + LvmBootdiskOptionsView::new_with_defaults(&runinfo.disks[0], &product_conf), ), - FsType::Zfs(_) => { - view.add_child(ZfsBootdiskOptionsView::new_with_defaults(disks)) - } + FsType::Zfs(_) => view.add_child(ZfsBootdiskOptionsView::new_with_defaults( + &runinfo, + &product_conf, + )), FsType::Btrfs(_) => { - view.add_child(BtrfsBootdiskOptionsView::new_with_defaults(disks)) + view.add_child(BtrfsBootdiskOptionsView::new_with_defaults(&runinfo.disks)) } } } @@ -179,7 +178,7 @@ impl AdvancedBootdiskOptionsView { 0, SelectView::new() .popup() - .with_all(disks.iter().map(|d| (d.to_string(), d.clone()))), + .with_all(runinfo.disks.iter().map(|d| (d.to_string(), d.clone()))), ); } other => view.replace_child(0, TextView::new(other.to_string())), @@ -547,8 +546,11 @@ impl ZfsBootdiskOptionsView { Self { view } } - fn new_with_defaults(disks: &[Disk]) -> Self { - Self::new(disks, &ZfsBootdiskOptions::defaults_from(disks)) + fn new_with_defaults(runinfo: &RuntimeInfo, product_conf: &ProductConfig) -> Self { + Self::new( + &runinfo.disks, + &ZfsBootdiskOptions::defaults_from(runinfo, product_conf), + ) } fn get_values(&mut self) -> Option<(Vec, ZfsBootdiskOptions)> { @@ -568,6 +570,7 @@ impl ZfsBootdiskOptionsView { compress, checksum, copies, + arc_max: 0, // use built-in ZFS default value disk_size, selected_disks, }, -- 2.42.0