On Sun, Nov 19, 2023 at 6:41 PM Chris Li <chrisl@xxxxxxxxxx> wrote:
>
> Hi Nhat,
>
> On Sun, Nov 19, 2023 at 01:50:17PM -0800, Chris Li wrote:
> > On Sun, Nov 19, 2023 at 11:08 AM Nhat Pham <nphamcs@xxxxxxxxx> wrote:
> > > I don't have any major argument against this. It just seems a bit
> > > heavyweight for what we need at the moment (only disabling
> > > swap-to-disk usage).
> >
> > The first milestone we just implement the reserved keywords without
> > the custom swap tier list.
> > That should be very similar to "zswap.writeback". Instead of writing 0
> > to "zswap.writeback".
> > You write "zswap" to "swap.tiers". Writing "none" will disable all
> > swap. Writing "all" will allow all swap devices.
> > I consider this conceptually cleaner than the "zswap.writeback" == 0
> > will also disable other swap types behavior. "disabled zswap writeback
> > == disable all swap" feels less natural.
>
> I implement a minimal version of the "swap.tiers" to replace the "zswap.writeback".
> It only implements the ABI level. Under the hook it is using the writeback bool.
>
> This patch builds on top of your V5 patch.
>
> implement memory.swap.tiers on top of memory.zswap.writeback.
>
> "memory.swap.tiers" supports two key words for now:
> all: all swap swap tiers are considered. (previously zswap.writback == 1)
> zswap: only zswap tier are considered. (previously zswap.writeback == 0)
>
> Index: linux/mm/memcontrol.c
> ===================================================================
> --- linux.orig/mm/memcontrol.c
> +++ linux/mm/memcontrol.c
> @@ -7992,6 +7992,32 @@ static int swap_events_show(struct seq_f
> return 0;
> }
>
> +static int swap_tiers_show(struct seq_file *m, void *v)
> +{
> + struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> +
> + seq_printf(m, "%s\n", READ_ONCE(memcg->zswap_writeback) ? "all" : "zswap");
> + return 0;
> +}
> +
> +static ssize_t swap_tiers_write(struct kernfs_open_file *of,
> + char *buf, size_t nbytes, loff_t off)
> +{
> + struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
> + int zswap_writeback;
> +
> + buf = strstrip(buf);
> + if (!strcmp(buf, "all"))
> + zswap_writeback = 1;
> + else if (!strcmp(buf, "zswap"))
> + zswap_writeback = 0;
> + else
> + return -EINVAL;
> +
> + WRITE_ONCE(memcg->zswap_writeback, zswap_writeback);
> + return nbytes;
> +}
> +
> static struct cftype swap_files[] = {
> {
> .name = "swap.current",
> @@ -8021,6 +8047,12 @@ static struct cftype swap_files[] = {
> .file_offset = offsetof(struct mem_cgroup, swap_events_file),
> .seq_show = swap_events_show,
> },
> + {
> + .name = "swap.tiers",
> + .seq_show = swap_tiers_show,
> + .write = swap_tiers_write,
> + },
> +
> { } /* terminate */
> };
>
> @@ -8183,31 +8215,6 @@ static ssize_t zswap_max_write(struct ke
> return nbytes;
> }
>
> -static int zswap_writeback_show(struct seq_file *m, void *v)
> -{
> - struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> -
> - seq_printf(m, "%d\n", READ_ONCE(memcg->zswap_writeback));
> - return 0;
> -}
> -
> -static ssize_t zswap_writeback_write(struct kernfs_open_file *of,
> - char *buf, size_t nbytes, loff_t off)
> -{
> - struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
> - int zswap_writeback;
> - ssize_t parse_ret = kstrtoint(strstrip(buf), 0, &zswap_writeback);
> -
> - if (parse_ret)
> - return parse_ret;
> -
> - if (zswap_writeback != 0 && zswap_writeback != 1)
> - return -EINVAL;
> -
> - WRITE_ONCE(memcg->zswap_writeback, zswap_writeback);
> - return nbytes;
> -}
> -
> static struct cftype zswap_files[] = {
> {
> .name = "zswap.current",
> @@ -8220,11 +8227,6 @@ static struct cftype zswap_files[] = {
> .seq_show = zswap_max_show,
> .write = zswap_max_write,
> },
> - {
> - .name = "zswap.writeback",
> - .seq_show = zswap_writeback_show,
> - .write = zswap_writeback_write,
> - },
> { } /* terminate */
> };
> #endif /* CONFIG_MEMCG_KMEM && CONFIG_ZSWAP */
Hi Chris!
Thanks for the patch. Would you mind if I spend some time staring
at the suggestion again and testing it some more?
If everything is good, I'll squash this patch with the original version,
(keeping you as a co-developer of the final patch of course), and
update the documentation before re-sending everything as v6.
Anyway, have a nice Thanksgiving break everyone! Thanks for
taking the time to review my patch and discuss the API with me!
