Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
From:   Barry Song <21cnbao@gmail.com>
To:     ryan.roberts@arm.com
Cc:     akpm@linux-foundation.org, anshuman.khandual@arm.com,
        catalin.marinas@arm.com, david@redhat.com, fengwei.yin@intel.com,
        hughd@google.com, itaru.kitayama@gmail.com, jhubbard@nvidia.com,
        kirill.shutemov@linux.intel.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, mcgrof@kernel.org, rientjes@google.com,
        shy828301@gmail.com, vbabka@suse.cz, wangkefeng.wang@huawei.com,
        willy@infradead.org, ying.huang@intel.com, yuzhao@google.com,
        ziy@nvidia.com
Subject: Re: [RESEND PATCH v7 02/10] mm: Non-pmd-mappable, large folios for folio_add_new_anon_rmap()
Date:   Mon, 27 Nov 2023 17:36:44 +1300
Message-Id: <20231127043644.8072-1-v-songbaohua@oppo.com>
In-Reply-To: <20231122162950.3854897-3-ryan.roberts@arm.com>
References: <20231122162950.3854897-3-ryan.roberts@arm.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

>  void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
>  		unsigned long address)
>  {
> -	int nr;
> +	int nr = folio_nr_pages(folio);
> 
> -	VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma);
> +	VM_BUG_ON_VMA(address < vma->vm_start ||
> +			address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
>  	__folio_set_swapbacked(folio);
> +	__folio_set_anon(folio, vma, address, true);
> 
> -	if (likely(!folio_test_pmd_mappable(folio))) {
> +	if (likely(!folio_test_large(folio))) {
>  		/* increment count (starts at -1) */
>  		atomic_set(&folio->_mapcount, 0);
> -		nr = 1;
> +		SetPageAnonExclusive(&folio->page);
> +	} else if (!folio_test_pmd_mappable(folio)) {
> +		int i;
> +
> +		for (i = 0; i < nr; i++) {
> +			struct page *page = folio_page(folio, i);
> +
> +			/* increment count (starts at -1) */
> +			atomic_set(&page->_mapcount, 0);
> +			SetPageAnonExclusive(page);

Hi Ryan,

we are doing an entire mapping, right? what is the reason to
increase mapcount for each subpage? shouldn't we only increase
mapcount of subpage in either split or doublemap case?

in page_add_anon_rmap(), are we also increasing mapcount of
each subpage for fork() case where the entire large folio
is inheritted by child processes?

> +		}
> +
> +		atomic_set(&folio->_nr_pages_mapped, nr);
>  	} else {
>  		/* increment count (starts at -1) */
>  		atomic_set(&folio->_entire_mapcount, 0);
>  		atomic_set(&folio->_nr_pages_mapped, COMPOUND_MAPPED);
> -		nr = folio_nr_pages(folio);
> +		SetPageAnonExclusive(&folio->page);
>  		__lruvec_stat_mod_folio(folio, NR_ANON_THPS, nr);
>  	}
> 
>  	__lruvec_stat_mod_folio(folio, NR_ANON_MAPPED, nr);
> -	__folio_set_anon(folio, vma, address, true);
> -	SetPageAnonExclusive(&folio->page);
>  }

Thanks
Barry