On Thu 25-07-24 13:38:50, Barry Song wrote:
> On Thu, Jul 25, 2024 at 12:17 AM Michal Hocko <mhocko@xxxxxxxx> wrote:
> >
> > On Wed 24-07-24 20:55:44, Barry Song wrote:
> > > From: Barry Song <v-songbaohua@xxxxxxxx>
> > >
> > > GFP_NOFAIL includes the meaning of block and direct reclamation, which
> > > is essential for a true no-fail allocation. We are gradually starting
> > > to enforce this block semantics to prevent the potential misuse of
> > > __GFP_NOFAIL in atomic contexts in the future.
> > >
> > > A typical example of incorrect usage is in VDPA, where GFP_ATOMIC
> > > and __GFP_NOFAIL are used together.
> >
> > Ohh, so you have done the migration. Please squash those two patches.
> > Also if we want to preserve clean __GFP_NOFAIL for internal MM use then it
> > should be moved away from include/linux/gfp_types.h. But is there any
> > real use for that?
>
> yes. currently i got two,
>
> lib/rhashtable.c
>
> static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
> size_t nbuckets,
> gfp_t gfp)
> {
> struct bucket_table *tbl = NULL;
> size_t size;
> int i;
> static struct lock_class_key __key;
>
> tbl = alloc_hooks_tag(ht->alloc_tag,
> kvmalloc_node_noprof(struct_size(tbl, buckets,
> nbuckets),
> gfp|__GFP_ZERO, NUMA_NO_NODE));
>
> size = nbuckets;
>
> if (tbl == NULL && (gfp & ~__GFP_NOFAIL) != GFP_KERNEL) {
> tbl = nested_bucket_table_alloc(ht, nbuckets, gfp);
> nbuckets = 0;
> }
>
> ...
>
> return tbl;
> }
Ugh. OK this is a weird allocation fallback strategy 2d22ecf6db1c
("lib/rhashtable: guarantee initial hashtable allocation"). Maybe the
code should be just simplified and GFP_NOFAIL used from the begining?
Davidlohr WDYT? For your context Barry tries to drop all the
__GFP_NOFAIL use and replace it by GFP_NOFAIL which enforces
__GFP_DIRECT_RECLAIM so that people cannot request atomic NOFAIL.
> and tools/perf/builtin-kmem.c:
>
> static const struct {
> const char *original;
> const char *compact;
> } gfp_compact_table[] = {
> { "GFP_TRANSHUGE", "THP" },
> { "GFP_TRANSHUGE_LIGHT", "THL" },
> { "GFP_HIGHUSER_MOVABLE", "HUM" },
> { "GFP_HIGHUSER", "HU" },
> ...
> { "__GFP_NOFAIL", "NF" },
> ...
> };
This is a prrintk formatting stuff. This counts as low level
functionality.
--
Michal Hocko
SUSE Labs
