On Wed, Feb 08, 2023 at 06:14:48PM +0000, Matthew Wilcox wrote:
> On Wed, Feb 08, 2023 at 11:38:48AM +0300, Kirill A. Shutemov wrote:
> > > +static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
> > > + pte_t *ptep, pte_t pte, unsigned int nr)
> > > +{
> > > + for (;;) {
> > > + set_pte_at(mm, addr, ptep, pte);
> > > + if (--nr == 0)
> > > + break;
> >
> > Maybe do { ... } while (--nr); instead of the for()?
>
> Seems unnecessary to do the extra calculation to advance pte if
> we're not going to use it?
It should be transparent to compiler, but okay.
>
> > > + ptep++;
> > > + addr += PAGE_SIZE;
> > > + /* This works for x86. Check how PTEs are encoded */
> > > + pte = __pte(pte_val(pte) + PAGE_SIZE);
> >
> > Looks like it deserves own helper. Something like
> >
> > pte = pte_next(pte);
>
> Maybe, but then it needs to be added to each arch, which I was trying
> to avoid. That said, I had another idea which I like better; more
> patches coming soon.
Generic pte_next() that does the "+ PAGE_SIZE" trick should be fine. Arch
code can override it if bit placement is different, instead of rewriting
whole set_ptes()'
> Here's a sneak preview:
>
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -1019,13 +1019,22 @@ static inline pud_t native_local_pudp_get_and_clear(pud_
> t *pudp)
> return res;
> }
>
> -static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
> - pte_t *ptep, pte_t pte)
> +static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, pte_t pte, unsigned int nr)
> {
> - page_table_check_ptes_set(mm, addr, ptep, pte, 1);
> - set_pte(ptep, pte);
> + page_table_check_ptes_set(mm, addr, ptep, pte, nr);
> +
> + for (;;) {
> + set_pte(ptep, pte);
> + if (--nr == 0)
> + break;
> + ptep++;
> + pte = __pte(pte_val(pte) + PAGE_SIZE);
> + }
for(; nr; nr--, ptep++, pte = pte_next(pte))
set_pte(ptep, pte);
Hm?
> }
>
> +#define set_pte_at(mm, addr, ptep, pte) set_ptes(mm, addr, ptep, pte, 1)
> +
> static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr,
> pmd_t *pmdp, pmd_t pmd)
> {
>
>
>
--
Kiryl Shutsemau / Kirill A. Shutemov
