all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Alexander Abraham <a.abraham@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-docs v3] fix #5644: Split the section on creating a swap partition on a ZVol.
Date: Thu, 6 Feb 2025 13:45:05 +0100	[thread overview]
Message-ID: <14c0c179-6ea7-439c-a9ef-77cdaf944d9e@proxmox.com> (raw)
In-Reply-To: <20250205124127.111180-1-a.abraham@proxmox.com>

Nit: we usually don't use a dot at the end of a commit title

Am 05.02.25 um 13:41 schrieb Alexander Abraham:
> The section "SWAP on ZFS" was split into two parts. The first part
> contains general instructions on how to create a swap partition and
> how to set the level of "swappiness". The second part contains a
> warning about creating a swap partition on a ZFS volume.The official
> ZFS documentation, which has been linked in a footnote, explains
> that creating a swap partition on a ZFS volume could lead to
> dangerous and undefined behavior.
> 
> Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>

Shaping up nicely now! A few more comments/nits below.

> ---
>  local-zfs.adoc | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/local-zfs.adoc b/local-zfs.adoc
> index c64fb27..d411222 100644
> --- a/local-zfs.adoc
> +++ b/local-zfs.adoc
> @@ -622,14 +622,9 @@ time this value changes:
>  You *must reboot* to activate these changes.
>  ====
>  
> -
> -[[zfs_swap]]
> -SWAP on ZFS
> -~~~~~~~~~~~
> -
> -Swap-space created on a zvol may generate some troubles, like blocking the
> -server or generating a high IO load, often seen when starting a Backup
> -to an external Storage.
> +[[creating_swap_partitions]]
> +Creating SWAP Partitions
> +~~~~~~~~~~~~~~~~~~~~~~~~
>  

This is not related to ZFS, so a second patch could move this section to
a different place, see Alexander Zeidler's suggestion in v2:
https://lore.proxmox.com/pve-devel/D7ITC3NDGSV5.1S3BV4L2Q49LF@proxmox.com/

>  We strongly recommend to use enough memory, so that you normally do not
>  run into low memory situations. Should you need or want to add swap, it is
> @@ -637,18 +632,18 @@ preferred to create a partition on a physical disk and use it as a swap device.
>  You can leave some space free for this purpose in the advanced options of the
>  installer. Additionally, you can lower the
>  ``swappiness'' value. A good value for servers is 10:
> -
> + 
>  ----
>  # sysctl -w vm.swappiness=10
>  ----
> -
> + 
>  To make the swappiness persistent, open `/etc/sysctl.conf` with
>  an editor of your choice and add the following line:
>  
>  --------
>  vm.swappiness = 10
>  --------
> -
> + 
>  .Linux kernel `swappiness` parameter values
>  [width="100%",cols="<m,2d",options="header"]
>  |===========================================================

Is there a specific reason behind these whitespace changes or were they
done by accident?

> @@ -663,6 +658,15 @@ improve performance when sufficient memory exists in a system.
>  | vm.swappiness = 100 | The kernel will swap aggressively.
>  |===========================================================
>  
> +[[zfs_swap]]
> +SWAP on ZFS
> +~~~~~~~~~~~
> +
> +It is strongly recommended to not use a ZFS volume for creating a swap partition

Missing a comma before "because"

> +because this could lead to deadlocks. These deadlocks could cause the affected 

Nit: I'd replace "could" with "can" both times

> +system to freeze. The OpenZFS documentation footnote:[https://openzfs.github.io/openzfs-docs/Project%20and%20Community/FAQ.html#using-a-zvol-for-a-swap-device-on-linux]

Nit: I'd move the footnote link on its own line, then it will overshoot
the line length a bit less.

> +warns about using a ZFS volume for creating a swap partition.
> +
>  [[zfs_encryption]]
>  Encrypted ZFS Datasets
>  ~~~~~~~~~~~~~~~~~~~~~~



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  reply	other threads:[~2025-02-06 12:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-05 12:41 Alexander Abraham
2025-02-06 12:45 ` Fiona Ebner [this message]
2025-05-14 16:49   ` [pve-devel] [PATCH pve-docs] fix #5644: Moved the section on ZFS Alexander Abraham
2025-05-20 11:56     ` Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=14c0c179-6ea7-439c-a9ef-77cdaf944d9e@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=a.abraham@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal