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: [PATCH v2 1/3] hugetlb: simplify prep_compound_gigantic_page ref
 count racing code
To:     Oscar Salvador <osalvador@suse.de>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Muchun Song <songmuchun@bytedance.com>,
        Michal Hocko <mhocko@suse.com>,
        David Hildenbrand <david@redhat.com>,
        Matthew Wilcox <willy@infradead.org>,
        Naoya Horiguchi <naoya.horiguchi@linux.dev>,
        Mina Almasry <almasrymina@google.com>,
        Andrew Morton <akpm@linux-foundation.org>
References: <20210809184832.18342-1-mike.kravetz@oracle.com>
 <20210809184832.18342-2-mike.kravetz@oracle.com>
 <19f47b787269b95bb76d81bb1e6bfcc3@suse.de>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <5d5a543a-026a-7b9b-30fd-47a31ac592d7@oracle.com>
Date:   Tue, 10 Aug 2021 09:51:03 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.1
In-Reply-To: <19f47b787269b95bb76d81bb1e6bfcc3@suse.de>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from [192.168.2.123] (50.38.35.18) by CO2PR06CA0060.namprd06.prod.outlook.com (2603:10b6:104:3::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.15 via Frontend Transport; Tue, 10 Aug 2021 16:51:05 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: bf783052-6b8b-4559-85ed-08d95c1f0b95
X-MS-TrafficTypeDiagnostic: BY5PR10MB4371:
X-Microsoft-Antispam-PRVS: <BY5PR10MB4371C5217D3BF33CBA122DBBE2F79@BY5PR10MB4371.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:9508;
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: 6rQ/dxuc5yEI9PhoWB2MnUniVimIslaqlXeSNLLaQUUJuqgH393qdwpYPPhfkoBcAM0VjdB7FRK32qWhYQdfuiow2iBAiP1ggkxhNx1gHnE8mtYOFCYUNE7VRKqHaE5jwsjTfyJ1ShSHgmTrN/vRxz5gG7du+i3RPGgDJKK1xz/zmPq2XwQaF7zgc4TB6WvkUxVyucHFAOo39koDnG3tjmUoAZpx9gDgSdRH4grETCD8hWMHWPiUzBjR0Kpxi3MWXCfyOU0bNhQ2WzBdJ0gKw7Ljw2+Id7DC/A/jZ0vTZ6EhqxAaHHaY53anEzFhMJ2R6IqISNO/giQPqLEzoL8JfjUQjrVcWpkuEcZpNzQt/EKDCwSy73aiBTm+teKwURXxyd+49HsZ8XgL96GooU8yn+MxMADAAzmrjVbHWe0WchVZcesOJZYzBesKS9PMdOroEOMAQgql9vyULBJyt3sdCcnn/mBViQiareXRY/pnAXROAP22iH0STLHmZLUqAtANb1nqD0kT7UuA9gB4+ZMrpMPCv0NMAzyeMWyzpNy4Ps8bCAq8kSBrTyPkvxlmVY+oNmzQGOl1DngShQ88CRlsa9SPMQ3hr6BqbFYJKiOuGdKwkjFf265PZchNN9d1By+OBvuduFfXr+1MlxTbHCtrCre9zZvNoptVSxnDmKgl2O9m9/7IghZQ2PxhyfP+x5QXnuDIUU0N+0NnVGFPYA+QHH+Qof1jJq1YTtem2EDG9PFQLosli8qiPjlRbaN4T8Ky447B/KliBzXTSSHCst4hHg==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BY5PR10MB4196.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(39860400002)(396003)(366004)(136003)(376002)(346002)(4326008)(31686004)(478600001)(66556008)(66476007)(66946007)(36756003)(26005)(2906002)(44832011)(52116002)(54906003)(2616005)(956004)(31696002)(53546011)(16576012)(86362001)(316002)(83380400001)(6916009)(186003)(8676002)(7416002)(38100700002)(5660300002)(8936002)(38350700002)(6486002)(45980500001)(43740500002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?L051KzVCbXEyWk1JSUdRNlN4Vk03eTNMcENWLzhpUUpqdldlUHhoZFkwMVZ4?=
 =?utf-8?B?d1BZVGtUQk1JV2V0SDhCSTZmdVZzb3pwc2tqdVZZaGhwWk9rekJPT3ZqTFkr?=
 =?utf-8?B?Qm5UUXU5cDI4bFhjYkwxM0VaY3lyOXJhRUJEclVaTzYrTXdBTHdsNjZPR3o5?=
 =?utf-8?B?T0tDL0d6anNZL1ZTT2RzZFZ4Ykd4TGxQQXBIN05oU1YyQS9YcCtHK3VEbnZ2?=
 =?utf-8?B?QlEwRGk1dlV0K0RaeDhpSjdxVW40YjVvS0RwUGRzVVVNRGZzYldJckhvQS9J?=
 =?utf-8?B?VWw2Tm1mU01iWWpTK3EycGU2ZVJzZVhaK2hMc01ySHdVWGM4c1FtZ2ZQYWhN?=
 =?utf-8?B?elM5TzdPeENjSGhWeEtWcWliQTVxd09lVGZuYlpmeGNDaGFpT2lhRnJ6TUQ2?=
 =?utf-8?B?VGZELzVXaTRkQU1LR01sNFBpVTRIVUhUYWlYbCs3SHJSSWRYRE16OEZNM2lH?=
 =?utf-8?B?OWF5dWZzV2NDRTBPbTBSVWxleGd0UjhGSnpGZytJUGNMYjhLSzRHT2xEQ2dh?=
 =?utf-8?B?SzlJeEFOL0R2b0tDT2dqV3diNGxzVCttR3NWWnNGVk4vTnhQdVh1S2FUSDhj?=
 =?utf-8?B?T2dXQ0xhTXBSSFY2ckt1TmloUlUwRHNmYU5BWGRsSkl6ek82UmxyYS9JbkY2?=
 =?utf-8?B?dlExdC9aUFBLTHBIUDNWckJjQmRoNUF6YVlNY2l1V1J5cGtpSTg5VVQ1MFFw?=
 =?utf-8?B?dlZzZXgxSkdtNC83bHN4R2tzeFNLRUlKSHdleGNGZm5iS3pWYTF0S3RYYmdm?=
 =?utf-8?B?cWl2VVdod3JjSlcyNGQ5UWg4dW5rbEZEanZmYXE4KzlMbjBLM0Yva1p4OHZx?=
 =?utf-8?B?TFRLUzRmLytFQXpLNDljeG1pMk92UHVGVDNSY1VsaFpSQmVOcmJWTlBmTEdS?=
 =?utf-8?B?VjBhRzg4YTd0R1Y4bnY5VmpSdVpWM0RUSmpMb2RtVHFLTHlYME5UWFg2TUg4?=
 =?utf-8?B?TElBODFuKzVSL0xkM3k5ci91Vks5RStsVTlvc1crVkcwZHluaGtsd0lKbGRS?=
 =?utf-8?B?NUk0QVJqSmFHR3BpczllMHdiVEVGMEZPN0RDU3hURVpTVDRqQ2duR2h2REJi?=
 =?utf-8?B?dkExb3ZPZHZGNzVKM3lZOG5qeDFVdFVtZXdpSU1tTEgxQ0lhUmpSTkpreUZX?=
 =?utf-8?B?dkF6K2VPS3RJUnpwdXJ3NTJlVkM5anh1VGR2alBTazZRYklMQ3hseDV2c2lU?=
 =?utf-8?B?akZ3cVVjb3NsMXFIL2VIREppZ1l6R1pCV3puaFI1N0ZGY0toTndnbjFnT2g5?=
 =?utf-8?B?NW40ck9CTU92b05OMTVmMHd4N28xa1QwL1gvcmRzUlJyQk14QkNEL0hYVk41?=
 =?utf-8?B?U2VoMVNIcllWM1VrRG9TZTVsSUlRbzRMWTh2a1hsN2hSMlZxMkU4VVMzYURR?=
 =?utf-8?B?Y0IwRkhUL293THVMbW9WMlExTllBQVA1Zzg5RFVUQjhmeEpycVJCVUFEa3lL?=
 =?utf-8?B?RmRjRXF5Z0hORHZPWXpkZmRZSzVZU1dRK3BqendKcmRIRm82L1loQzJuZW5F?=
 =?utf-8?B?dGtwQTRUbXl3bjVPWElVbTRNdVFjRTRFbEZDNXBLM2IvUEgrOHdqd3U3STVQ?=
 =?utf-8?B?T1MreUtOZHRsK2RaVUR0Mng3d29pekFPTksvUHg2alFLdmhDYkNOSnJWVGd3?=
 =?utf-8?B?SWJ4eklLU2pHb2N6NE1Nc2grREVhSks2VHlLdUZiSUJUQlE0UW1CT2JxVGlP?=
 =?utf-8?B?VUJxbzNkVTUrMjAvd3Y0NGJVeVI5Z0NLMTRxWFdCdlBrVkc2VGdOM1BxcEJM?=
 =?utf-8?Q?176KK0WYKP3zUffS4mcBojnesV5vJZOJD7TANAE?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bf783052-6b8b-4559-85ed-08d95c1f0b95
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Aug 2021 16:51:05.9557
 (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: tQBg0uTwIxvw93URCmIcHo09PSDU7Jm6h05s4AwdlGVS6r8WthPRwJMB+3gMaM2SqEP8270wTnhOEoI8Tf8kbQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4371
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10072 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999
 malwarescore=0 phishscore=0 bulkscore=0 suspectscore=0 mlxscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2107140000 definitions=main-2108100108
X-Proofpoint-ORIG-GUID: uX_qZnNjeTfzoOPfhL9z8whRmzZCb0pB
X-Proofpoint-GUID: uX_qZnNjeTfzoOPfhL9z8whRmzZCb0pB
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/10/21 2:29 AM, Oscar Salvador wrote:
> On 2021-08-09 20:48, Mike Kravetz wrote:
>> Code in prep_compound_gigantic_page waits for a rcu grace period if it
>> notices a temporarily inflated ref count on a tail page.  This was due
>> to the identified potential race with speculative page cache references
>> which could only last for a rcu grace period.  This is overly complicated
>> as this situation is VERY unlikely to ever happen.  Instead, just quickly
>> return an error.
>> Also, only print a warning in prep_compound_gigantic_page instead of
>> multiple callers.
> 
> The above makes sense to me.

Thanks for taking a look!

> My only question would be: gather_bootmem_prealloc() is an __init function.
> Can we have speculative refcounts due to e.g: pagecache at that early stage?
> I think we cannot, but I am not really sure.

I had the same thought when adding that code.  We cannot have a
speculative refcount this early after boot.  However, further
thinking below...

> 
> We might be able to remove that else() in case we cannot have such scenarios.
> 

Instead of removing the else, I think we should put a BUG_ON() just to
make sure the condition never happens in the future.  Otherwise, we would
just abandon the gigantic page and leave memory (1GB or so) unavailable.

Even if it were possible to have speculative references this early in
the boot process, the likelihood of it happening here is still VERY
small.  So, I would not expect a BUG_ON() to ever be hit in development or
testing.  Rather, with our luck it would show up in some production
environment.

Since handling the race in this routine is so simple, I chose to just
add the code to handle it.
-- 
Mike Kravetz