On 6/28/23 7:03 AM, Ritesh Harjani (IBM) wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxx> writes:
....
>> +int __meminit vmemmap_check_pmd(pmd_t *pmd, int node,
>> + unsigned long addr, unsigned long next)
>> +{
>> + int large = pmd_large(*pmd);
>> +
>> + if (pmd_large(*pmd))
>
> we already got the value of pmd_large into "large" variable.
> we can use just if (large) right?
>
>> + vmemmap_verify((pte_t *)pmd, node, addr, next);
>
> maybe we can use pmdp_ptep() function here which we used in the 1st patch?
> also shouldn't this be pmdp in the function argument instead of pmd?
>
updated
>> +
>> + return large;
>> +}
>> +
>> +void __meminit vmemmap_set_pmd(pmd_t *pmdp, void *p, int node,
>> + unsigned long addr, unsigned long next)
>> +{
>> + pte_t entry;
>> + pte_t *ptep = pmdp_ptep(pmdp);
>> +
>> + VM_BUG_ON(!IS_ALIGNED(addr, PMD_SIZE));
>> + entry = pfn_pte(__pa(p) >> PAGE_SHIFT, PAGE_KERNEL);
>> + set_pte_at(&init_mm, addr, ptep, entry);
>> + asm volatile("ptesync": : :"memory");
>> +
>> + vmemmap_verify(ptep, node, addr, next);
>> +}
>> +
>> +static pte_t * __meminit radix__vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
>> + struct vmem_altmap *altmap,
>> + struct page *reuse)
>> +{
>> + pte_t *pte = pte_offset_kernel(pmd, addr);
>> +
>> + if (pte_none(*pte)) {
>> + pte_t entry;
>> + void *p;
>> +
>> + if (!reuse) {
>> + /*
>> + * make sure we don't create altmap mappings
>> + * covering things outside the device.
>> + */
>> + if (altmap && altmap_cross_boundary(altmap, addr, PAGE_SIZE))
>> + altmap = NULL;
>> +
>> + p = vmemmap_alloc_block_buf(PAGE_SIZE, node, altmap);
>> + if (!p) {
>> + if (altmap)
>> + p = vmemmap_alloc_block_buf(PAGE_SIZE, node, NULL);
>> + if (!p)
>> + return NULL;
>> + }
>
> Above if conditions are quite confusing when looking for the 1st time?
> Can we do this? Did I get it right?
>
> if (!p && altmap)
> p = vmemmap_alloc_block_buf(PAGE_SIZE, node, NULL);
>
> if (!p)
> return NULL;
>
updated
-aneesh
