| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20251230024118.5263-1-lizhe.67@bytedance.com>
Date: Tue, 30 Dec 2025 10:41:16 +0800
From: "Li Zhe" <lizhe.67@...edance.com>
To: <fvdl@...gle.com>
Cc: <akpm@...ux-foundation.org>, <david@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
<lizhe.67@...edance.com>, <muchun.song@...ux.dev>, <osalvador@...e.de>
Subject: Re: [PATCH 4/8] mm/hugetlb: introduce per-node sysfs interface "zeroable_hugepages"
> On Mon, 29 Dec 2025 10:57:23 -0800, fvdl@...gle.com wrote:
>
> On Mon, Dec 29, 2025 at 4:26 AM Li Zhe <lizhe.67@...edance.com> wrote:
> >
> > On Fri, 26 Dec 2025 10:51:01 -0800, fvdl@...gle.com wrote:
> >
> > > > +static ssize_t zeroable_hugepages_show(struct kobject *kobj,
> > > > + struct kobj_attribute *attr, char *buf)
> > > > +{
> > > > + struct hstate *h;
> > > > + unsigned long free_huge_pages_zero;
> > > > + int nid;
> > > > +
> > > > + h = kobj_to_hstate(kobj, &nid);
> > > > + if (WARN_ON(nid == NUMA_NO_NODE))
> > > > + return -EPERM;
> > > > +
> > > > + free_huge_pages_zero = h->free_huge_pages_node[nid] -
> > > > + h->free_huge_pages_zero_node[nid];
> > > > +
> > > > + return sprintf(buf, "%lu\n", free_huge_pages_zero);
> > > > +}
> > > > +
> > > > +static inline bool zero_should_abort(struct hstate *h, int nid)
> > > > +{
> > > > + return (h->free_huge_pages_zero_node[nid] ==
> > > > + h->free_huge_pages_node[nid]) ||
> > > > + list_empty(&h->hugepage_freelists[nid]);
> > > > +}
> > > > +
> > > > +static void zero_free_hugepages_nid(struct hstate *h,
> > > > + int nid, unsigned int nr_zero)
> > > > +{
> > > > + struct list_head *freelist = &h->hugepage_freelists[nid];
> > > > + unsigned int nr_zerod = 0;
> > > > + struct folio *folio;
> > > > +
> > > > + if (zero_should_abort(h, nid))
> > > > + return;
> > > > +
> > > > + spin_lock_irq(&hugetlb_lock);
> > > > +
> > > > + while (nr_zerod < nr_zero) {
> > > > +
> > > > + if (zero_should_abort(h, nid) || fatal_signal_pending(current))
> > > > + break;
> > > > +
> > > > + freelist = freelist->prev;
> > > > + if (unlikely(list_is_head(freelist, &h->hugepage_freelists[nid])))
> > > > + break;
> > > > + folio = list_entry(freelist, struct folio, lru);
> > > > +
> > > > + if (folio_test_hugetlb_zeroed(folio) ||
> > > > + folio_test_hugetlb_zeroing(folio))
> > > > + continue;
> > > > +
> > > > + folio_set_hugetlb_zeroing(folio);
> > > > +
> > > > + /*
> > > > + * Incrementing this here is a bit of a fib, since
> > > > + * the page hasn't been cleared yet (it will be done
> > > > + * immediately after dropping the lock below). But
> > > > + * it keeps the count consistent with the overall
> > > > + * free count in case the page gets taken off the
> > > > + * freelist while we're working on it.
> > > > + */
> > > > + h->free_huge_pages_zero_node[nid]++;
> > > > + spin_unlock_irq(&hugetlb_lock);
> > > > +
> > > > + /*
> > > > + * HWPoison pages may show up on the freelist.
> > > > + * Don't try to zero it out, but do set the flag
> > > > + * and counts, so that we don't consider it again.
> > > > + */
> > > > + if (!folio_test_hwpoison(folio))
> > > > + folio_zero_user(folio, 0);
> > > > +
> > > > + cond_resched();
> > > > +
> > > > + spin_lock_irq(&hugetlb_lock);
> > > > + folio_set_hugetlb_zeroed(folio);
> > > > + folio_clear_hugetlb_zeroing(folio);
> > > > +
> > > > + /*
> > > > + * If the page is still on the free list, move
> > > > + * it to the head.
> > > > + */
> > > > + if (folio_test_hugetlb_freed(folio))
> > > > + list_move(&folio->lru, &h->hugepage_freelists[nid]);
> > > > +
> > > > + /*
> > > > + * If someone was waiting for the zero to
> > > > + * finish, wake them up.
> > > > + */
> > > > + if (waitqueue_active(&h->dqzero_wait[nid]))
> > > > + wake_up(&h->dqzero_wait[nid]);
> > > > + nr_zerod++;
> > > > + freelist = &h->hugepage_freelists[nid];
> > > > + }
> > > > + spin_unlock_irq(&hugetlb_lock);
> > > > +}
> > >
> > > Nit: s/nr_zerod/nr_zeroed/
> >
> > Thank you for the reminder. I will address this issue in v2.
> >
> > > Feels like the list logic can be cleaned up a bit here. Since the
> > > zeroed folios are at the head of the list, and the dirty ones at the
> > > tail, and you start walking from the tail, you don't need to check if
> > > you circled back to the head - just stop if you encounter a prezeroed
> > > folio. If you encounter a prezeroed folio while walking from the tail,
> > > that means that all other folios from that one to the head will also
> > > be prezeroed already.
> >
> > Thank you for the thoughtful suggestion. Your line of reasoning is,
> > in most situations, perfectly valid. Under extreme concurrency,
> > however, a corner case can still appear. Imagine two processes
> > simultaneously zeroing huge pages: Process A enters
> > zero_free_hugepages_nid(), completes the zeroing of one huge page,
> > and marks the folio in the list as pre-zeroed. Should Process B enter
> > the same function moments later and decide to exit as soon as it
> > meets a prezeroed folio, the intended parallel zeroing would quietly
> > fall back to a single-threaded pace.
>
> Hm, setting the prezeroed bit and moving the folio to the front of the
> free list happens while holding hugetlb_lock. In other words, if you
> encounter a folio with the prezeroed bit set while holding
> hugetlb_lock, it will always be in a contiguous stretch of prezeroed
> folios at the head of the free list.
>
> Since the check for 'is this already prezeroed' is done while holding
> hugetlb_lock, you know for sure that the folio is part of a list of
> prezeroed folios at the head, and you can stop, right?
Sorry for the confusion earlier. You're right, this does make
zero_free_hugepages_nid() simpler. I'll update it in v2.
Thanks,
Zhe
Powered by blists - more mailing lists