Re: [PATCH v4 01/15] genpt: Generic Page Table base API

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Aug 27, 2025 at 12:11:40AM -0700, Randy Dunlap wrote:
> > +/**
> > + * pt_entry_num_contig_lg2() - Number of contiguous items for this leaf entry
> > + * @pts: Entry to query
> > + *
> > + * Returns the number of contiguous items this leaf entry spans. If the entry is
> 
>  * Returns:

I think I prefer to leave many of these as is, putting the entire body
in a Returns: block seems too weird. I did the ones that seemed to fit
that pattern.

Most of these descriptions are entirely talking about the return value
since that is the only thing the function does.

> > + * If true the caller use at level 0 pt_install_leaf_entry(PAGE_SHIFT). This is
> 
>                          uses
> ?
> although it might just be missing a word or two? I can't tell.

 * If true the caller can use, at level 0, pt_install_leaf_entry(PAGE_SHIFT).
 * This is useful to create optimized paths for common cases of PAGE_SIZE
 * mappings.

> > + * Otherwise the bit in position pt_table_item_lg2sz() should be set indicating
> > + * that a non-contigous singe item leaf entry is supported. The following
> 
>              non-contiguous
> Also, is that               single
> ?
> or is "singe" a real word here? (IDK.)

 * Otherwise the bit in position pt_table_item_lg2sz() should be set indicating
 * that a non-contiguous single item leaf entry is supported. The following

> > +enum {
> > +	PT_VADDR_MAX = sizeof(pt_vaddr_t) == 8 ? U64_MAX : U32_MAX,
> > +	PT_VADDR_MAX_LG2 = sizeof(pt_vaddr_t) == 8 ? 64 : 32,
> > +	PT_OADDR_MAX = sizeof(pt_oaddr_t) == 8 ? U64_MAX : U32_MAX,
> > +	PT_OADDR_MAX_LG2 = sizeof(pt_oaddr_t) == 8 ? 64 : 32,
> > +};
> 
> Hm, duplicated enum entry values?
> Interesting.

Nope one letter different, pt_vaddr_t != pt_oaddr_t in all cases.

> > + *  start/end
> > + *     An open range, eg [0,0) refers to no VA.
> 
> 	                 e.g.,
> 
> and is a half-open (or right-open) range or interval, not open.
> 
> Open would be (0, 0).
> Closed would be [0, 0].
> I used to think that was "clopen" but now I read that clopen refers
> to sets and not intervals.

Ok

> > +/*
> > + * Add index_count_lg2 number of entries to pts's VA and index. The va will be
> 
> s/VA/va/ for consistency?
> since it ("va") is defined in Generic Page Table Language.

I changed the language section to be upper case and fixed the lower
case versions I noticed. There are more upper case versions than
lower..

Got everything else, thanks a lot!

Jason