Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC 2/8] mm/hugetlb: remove useless set_page_count()
To:     Pasha Tatashin <pasha.tatashin@soleen.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        linux-m68k@lists.linux-m68k.org, anshuman.khandual@arm.com,
        willy@infradead.org, akpm@linux-foundation.org,
        william.kucharski@oracle.com, vbabka@suse.cz, geert@linux-m68k.org,
        schmitzmic@gmail.com, rostedt@goodmis.org, mingo@redhat.com,
        hannes@cmpxchg.org, guro@fb.com, songmuchun@bytedance.com,
        weixugc@google.com, gthelen@google.com
References: <20211026173822.502506-1-pasha.tatashin@soleen.com>
 <20211026173822.502506-3-pasha.tatashin@soleen.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <57fbf167-b2e8-69f6-7f79-8e989162c889@oracle.com>
Date:   Tue, 26 Oct 2021 11:44:17 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.1
In-Reply-To: <20211026173822.502506-3-pasha.tatashin@soleen.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TnQ2VWV2UTd6Qm5sWnNhdi9aMEZoc0ZydHV6U3IvYzlqM1VPTjU1Sm9CVG5n?=
 =?utf-8?B?RVlrKzZNZnBUSTVlUkRKeUl4bjBlN25wNTVRbVBjOXlzNkU2UDhPYUtvWEE0?=
 =?utf-8?B?WEhIZnJIcnlqS25Uc2M3SlM0MkdBd0w3UDVmbTFVNHh3Uit2c2d2bUlJaFNr?=
 =?utf-8?B?VHdrNkdFSXVjKzYxdHZWNXhpbndhdS8xa2pobzVzWXF2MXE1cDM0bVRtUW0w?=
 =?utf-8?B?ZmhGUEpSQkVNRVJDVmdHd0VIejZUTU1BYU1tTU9vTzVYTmtZSzZwS01xUlg1?=
 =?utf-8?B?S2psdTFuQ0ovN2I1eGdJMW1yTUN0L2djQTRObXcrcDBybmZiOXdyeFUvcVhv?=
 =?utf-8?B?Um9XVm9JWDk0WlJsZVA1MXpIK0treDdzZ0Q1S0Z2REljeUlFNGZNbm0yRzlX?=
 =?utf-8?B?NnVpc0xITjFXWU80cjNPL1F6SmhBcFhwRmpZZGlRZ2tMTklqeGFLS2NWMDh3?=
 =?utf-8?B?VmJDYTQ3VmRJRWhiUGFyaUhMcHc5OG9oMUk0UWVnUEdPbmxEMFlNTUJ2cHJZ?=
 =?utf-8?B?d1NLRFd6cmVOT0RLQ1FVYnVKb2dNYWgxdVVrWW5vQmV2ZkowTXhMZWsxOWp5?=
 =?utf-8?B?c3BRenlINVVTcnVSNFNNbzVZSE00MEwyTzFKeFFnc0JDaWFXZUJFSlF2ZUdC?=
 =?utf-8?B?ZUhNTDFnMUJ1emNQcC9oVzF2cU94Z0dnbVAxUzMxUUVsN1ZSbWVsSjlFTFRN?=
 =?utf-8?B?Y1FoV0pKOGFLd2pnZVdPTnR6bEZpMkM4eUtGTHFDOXUxV1ZWT2h0VFZiUHNB?=
 =?utf-8?B?N2dYbzYwbjBCaHNUd1ZhejFEMGo4QXhNb0w0cExyYnN1Tmc5WkV5NnNhN1NG?=
 =?utf-8?B?SmVXcWFYWTVWaHg4VXl3QzR0b0JNMkwxcko2TU1ueEJlbjVXSkhuMDFINnh5?=
 =?utf-8?B?SFA2cm82QVMvd2dzTlhTOUQyWldhVW93TW1VOUpDZWpsRiszTUJSSVBPRzhE?=
 =?utf-8?B?b0s5aGVtM0gzRzZZamFVdHA5V01NRHZqeEQ4TWNEUXJybUFaclJwalRiejZu?=
 =?utf-8?B?Qm9rUEQ5dkNtZXRLMXlGL2RxYnU1NGxoNzBjd25UUkJRdGxYT05IUThORGtH?=
 =?utf-8?B?ZkRUNFJnSittREQ2dlY1ZlhTMGZ0RlEzaXVkUUR1SzBXcmZKdHkzb1RHOHpV?=
 =?utf-8?B?MEZ1UEFZMi9ybjlBVlZVVG1FdVZaWUcxVVFTemdxUFdxNi9iUU83T0RIY283?=
 =?utf-8?B?bGhINDdSaXZsYlhXbGo5VzFmWVNFNkZabUo5alZSQjNNbWN4V2xwaDZwTHZD?=
 =?utf-8?B?V0YyTHhzV0svZnpnckxqcFRZZUw2bHNJcjlISlVPZkRud2k1TmRoTjlZQjhR?=
 =?utf-8?B?M0cxN0hxRXRwNFkrSlRNdXJiVVVCRTlRb1hiYVZkSzNLMmVZeUFsVG1VdHlw?=
 =?utf-8?B?MEIvZXJIdU9jaEFzaGZRM3U5NWZ4MGJxdGhhR05uQko1c3RadDFNbFdtWlB4?=
 =?utf-8?B?QUkyTUM5Y3JIM1JwUjUxZ2ZXRTRKaVRPQTNoUFBORUdPeUdvM3Yybk9iaHF2?=
 =?utf-8?B?YllQQ3VHa2lPczJIRU5tQXBwN2gwaGJxOW9NRTBldTkrczRYMUZKQ2NBeDU0?=
 =?utf-8?B?V1pVdlVlUXFoZDFoVVJsY0s1ZTRWUVJuOU5EQkwxcTYrbzlreVpxdTltRm9T?=
 =?utf-8?B?cE9SYjNtM0hFNEpXaGtOR2N1Z21HdlpxMURKaUJQWW0zRmxjRWVhYnRCWXVh?=
 =?utf-8?B?c3o3VFpyTkFMWEp1VDJpQzZlRC9qYzdwVGc1djY4c1BBaDJVZm9jOEUrTGVx?=
 =?utf-8?B?OHJhMDVsKzE4UWRVYWlGSjBTcnEyZmNKTEpjNE5FTDVvbnpiZUN0Rm5GLzVD?=
 =?utf-8?B?NkhZTnl4ZnRFZzArWVV2Y3ZqOThMOWJFUnJRTklveUV0U3J4dGpGMGNGVnZ0?=
 =?utf-8?B?RUZXQ0p1SDJCVUhrMEZtNURwNnZYYmsyQ3VlS2dCckRJMTlwemI1RjY3SFBT?=
 =?utf-8?B?dWhtUk9SOWY1bHpqSm1VK0J4Q1dxeUdNWnNsOWJCdCs1elpybnpldjlxVDZz?=
 =?utf-8?B?OU1xbm5zWGJqcGVhMEdFam5DWVNyY000bFRqS0t3ZXlCSnBJN3FDcGQzVTRn?=
 =?utf-8?B?U1dUYVcwQktQT01IbHRCbld0a2pPOEl4akJNS1J2WUFJRnh0R2FNVnF2WVIz?=
 =?utf-8?B?SnYyQ2F1STdVUVVyMmxzNDdraUVzcUkzdGlqbnhxaHZueEN1ME9yWU5HampY?=
 =?utf-8?Q?MLX0PSuQHALKRROFqh/1alc=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cad56cf4-5f35-4131-f9f4-08d998b09e99
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2021 18:44:19.3397
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: KD84o4kg4hSYyKrf1b6iFkdE936wWBgVa3OmuUidHzET+a6Bwt1iwwTac7mmgW9s4G1EqZWtWeKfBikWifhpBQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3734
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10149 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 malwarescore=0
 adultscore=0 suspectscore=0 bulkscore=0 mlxscore=0 spamscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2110260103
X-Proofpoint-GUID: 0Y0IpQMD7SxAJLQe1CiAhJMgJfeUgN3k
X-Proofpoint-ORIG-GUID: 0Y0IpQMD7SxAJLQe1CiAhJMgJfeUgN3k
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/26/21 10:38 AM, Pasha Tatashin wrote:
> prep_compound_gigantic_page() calls set_page_count(0, p), but it is not
> needed because page_ref_freeze(p, 1) already sets refcount to 0.
> 
> Using, set_page_count() is dangerous, because it unconditionally resets
> refcount from the current value to unrestrained value, and therefore
> should be minimized.
> 
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Thanks!

My bad for not removing the set_page_count when adding the page_ref_freeze.

FYI, there have been additional changes to this routine in Andrew's
tree.  Not really sure if we want/need the VM_BUG_ON_PAGE as that would
only check if there was a 'bug' in page_ref_freeze.

-- 
Mike Kravetz

> ---
>  mm/hugetlb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 95dc7b83381f..7e3996c8b696 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1707,7 +1707,7 @@ static bool prep_compound_gigantic_page(struct page *page, unsigned int order)
>  			pr_warn("HugeTLB page can not be used due to unexpected inflated ref count\n");
>  			goto out_error;
>  		}
> -		set_page_count(p, 0);
> +		VM_BUG_ON_PAGE(page_count(p), p);
>  		set_compound_head(p, page);
>  	}
>  	atomic_set(compound_mapcount_ptr(page), -1);
>