Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 10 Nov 2022 14:31:27 -0800
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Nadav Amit <nadav.amit@gmail.com>
Cc:     Linux-MM <linux-mm@kvack.org>,
        kernel list <linux-kernel@vger.kernel.org>,
        Naoya Horiguchi <naoya.horiguchi@linux.dev>,
        David Hildenbrand <david@redhat.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Mina Almasry <almasrymina@google.com>,
        Peter Xu <peterx@redhat.com>, Rik van Riel <riel@surriel.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Matthew Wilcox <willy@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Wei Chen <harperchen1110@gmail.com>, stable@vger.kernel.org
Subject: Re: [PATCH v8 1/2] hugetlb: don't delete vma_lock in hugetlb
 MADV_DONTNEED processing
Message-ID: <Y217vw+cMjXcT3aq@monkey>
References: <20221108011910.350887-1-mike.kravetz@oracle.com>
 <20221108011910.350887-2-mike.kravetz@oracle.com>
 <9BB0EA0C-6E7C-462B-8374-5BFEC34E8415@gmail.com>
 <Y21xlgG6sxP6q5K9@monkey>
 <05FB2C99-9141-4F2A-8664-31CA5587B310@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <05FB2C99-9141-4F2A-8664-31CA5587B310@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SjhONGw1WTJEME9VWDZNanZaelRmSDc1RytHR01Qa0NpSjlCeDJKUUpHNXB6?=
 =?utf-8?B?bTh5bHFuMDVWTTR1WHFVZURDZVR6RHlpVFhiRXRrZjhmOWR0MW5sNWJaWnFC?=
 =?utf-8?B?QmtoNHJjS1NFZDdiZFpjMkFZVnUvQmg2Q3JmL2c3ekYxTWJITmJLbUp2NFBn?=
 =?utf-8?B?RGM1NzJzcVUvR0pWYjQ1M2llQzFQUHF1Ym5jT3MvM0xkWkYvbndKTUI2QzRs?=
 =?utf-8?B?Uzd0QW5iNG01U3hNdXd1OGNXUm0vVHcxMHg3eWNEc1ZkR1hjTDc0UHpUNVpF?=
 =?utf-8?B?d051L3ZBdzJPQUt4Tk1rUVJGTEZ2aVoxdUx4Z0h4OXRUcTVCdGYxTXc0YU12?=
 =?utf-8?B?TEUzVG15YkswU05vQjNMMUNCYVN6N3B5SnVPa3NFcHdrejFuSFZZUnhGR3E1?=
 =?utf-8?B?NkFnNmxTYXZHNWRwWkhacG4rQjZVMDFjejVhZU95dGY3YllUMzBSWTF6aC9Q?=
 =?utf-8?B?U2FUcVBXWnBGcWtNanh4dy9BdlVkWFRFQlVjaUNzbXUwMWFITjJidit0a0pY?=
 =?utf-8?B?TzQ3djgwU01DcXhtQnM4WjJaUTVXN29lUXN1TVpzYmc1eXI1djg4MHM2ZUl1?=
 =?utf-8?B?c05WWkF6REp1WUxsZlVtSU04V3g1WmVwcE1qMkdqMERFODRXRFVWK0FHbUJQ?=
 =?utf-8?B?UTlZUDAwWkMwaTZqTm96UlpFaFZ4Q2pRK3VzRk1ZY1AwT3hOeDR1RzhNRGVN?=
 =?utf-8?B?QmpYMHdJZlh6aW9xeHppQ2s0Q3FBSkJ4MXZTUzJoZng1UTJUYkNLU1B1U2dO?=
 =?utf-8?B?Zzhabm1ZV2Z2dU9VU2JNMnEyT0V2c282ZCtQck5zeCtXc21OS3lxMWw3WGVa?=
 =?utf-8?B?UnJtdFFYRmhtSnhmZFpPZStLb2QrekNIOTR1Rm9UQi9KNGRqcjZ3ZlFraHB3?=
 =?utf-8?B?cEJtWFZBWm51Yy90cnA4WXJBV3VjdVdHQ2pWdlNQZVFKT1JWcUsvam9oblhu?=
 =?utf-8?B?VGhyZGVLaHR4dUtBUENHYncrOXcwd1FSaHZ3OEQ3MVVLaEpWS0UwZTBxOWR0?=
 =?utf-8?B?alR2bU9KT2JOY0hrdFFGM1NiT3FSckI3dGZVaHhtYlMyejQ1L3J6alY4WG5C?=
 =?utf-8?B?SVF0SVA0YndNNzB5Znh3cmpiM1BWWXdoU3Qza3pVWjgxeUZBdVZlRFdHZVJu?=
 =?utf-8?B?dkpYZVdLclU5dklmRFc5ZFR3U2MyOUJBdEVMMGNWejFWRk1pbEpVQVZ5Y2cx?=
 =?utf-8?B?MHRyWlF6SzZkZ2xaTEpJRHBlVXJKblZGb0FSZ0VIZWVidVdZdFNZdi9Sc1lL?=
 =?utf-8?B?MXg1OEpIMlJpL3l4dDJRS1dIcEh2UGNPSW5SME9aUlhYYUxsZDNkMWcveitL?=
 =?utf-8?B?ZEFHSmRhelZKUkxXbTR4dHdpYjhlczY0dFdwMEJURnFtbXYvakFRZ3U2ZkRR?=
 =?utf-8?B?WFg1eW1RdHYxT0JuYXVYZWJGTGpvOEtjcUY0NlVvTTUxejM3VHNBQnZJNDR3?=
 =?utf-8?B?ZDFxWjlJc0NEUzF5N0E4c0hQckpkWWVqdjFrTFJWanMwY0s4WFQyUkhWajFI?=
 =?utf-8?B?LzB4SVVXWStham9UdnFvUVMyekJNV1ljS25GMGZiUkJmdGxERWY3YlJ5WWs2?=
 =?utf-8?B?bnpSVWdxdzdCaHdqV1MyRzRucG5NNTJvQ2FOWEp4dlUybHJZRU5lOGdTbjFR?=
 =?utf-8?B?bkp2Z3hjMzI2TTE2SGp2YzBpUCtQSkVValBwd1pGY0NOZTR1TWNPTlp0cTl1?=
 =?utf-8?B?MlhZQnBJdUtFbzZVcmlFQkxXNURnK0VUSHdnM1VKelNDK0w2bFRWYnYzZnY4?=
 =?utf-8?B?ZnNFOGYzYnVrRkFHWUQ4bTRhN0dnd2tIVlNCczRXcUhQR09MQWFkbWNLR3VH?=
 =?utf-8?B?QU1TV2NDWmlBR0VmWE5QNm9ud01jSWF1UC8yR3hJQUtyc2hQVW83eVBWdHRH?=
 =?utf-8?B?N05TQWNrWU91eVZGRGhVdytEU0lpOXBHaGNlSzduazNDOWx5dzlHV3hiTkRH?=
 =?utf-8?B?ZXNUSEE5RXZocGo4SDJDdHV5cUUzL01zckQrYldYa0x2ZWcrZzdsTyt4QVFm?=
 =?utf-8?B?aFQ2NnlKM2doNWFWVmhTRXZ5NTJkSm05NjR0V29WcWJkMEoxUEk0RFkwUVlL?=
 =?utf-8?B?T0pQbDc5SEtzOEFZd2hSSDdUWm91Uk0zOTk4K1VYTHI1cUZUMm9POGh0ZkJo?=
 =?utf-8?B?NjdOTzN4UFBjSTdqUTVlY0ZUR1BoMU9EWExRMDZ4S0JmYTVNU2crWmkyK05Y?=
 =?utf-8?B?S0E9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1ac95c1f-3471-4408-e66e-08dac36b50a0
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2022 22:31:30.9703
 (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: mD7QGvOLOxTVcMVU6qgPKWB55ydZr956LGYTXATz3BEF++a7v+K10mCMeF0JEFcN58E0DuScoE+x6mYudPTeJg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR10MB4585
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-10_14,2022-11-09_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxscore=0 spamscore=0
 adultscore=0 malwarescore=0 mlxlogscore=999 bulkscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211100158
X-Proofpoint-ORIG-GUID: tKtT4Qjj8DqQON2XZxDG29c9uBAzkpCq
X-Proofpoint-GUID: tKtT4Qjj8DqQON2XZxDG29c9uBAzkpCq
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/10/22 14:22, Nadav Amit wrote:
> On Nov 10, 2022, at 1:48 PM, Mike Kravetz <mike.kravetz@oracle.com> wrote:
> 
> >>> void unmap_hugepage_range(struct vm_area_struct *vma, unsigned long start,
> >>> 			  unsigned long end, struct page *ref_page,
> >>> 			  zap_flags_t zap_flags)
> >>> {
> >>> +	struct mmu_notifier_range range;
> >>> 	struct mmu_gather tlb;
> >>> 
> >>> +	mmu_notifier_range_init(&range, MMU_NOTIFY_UNMAP, 0, vma, vma->vm_mm,
> >>> +				start, end);
> >>> +	adjust_range_if_pmd_sharing_possible(vma, &range.start, &range.end);
> >>> 	tlb_gather_mmu(&tlb, vma->vm_mm);
> >>> +
> >>> 	__unmap_hugepage_range(&tlb, vma, start, end, ref_page, zap_flags);
> >> 
> >> Is there a reason for not using range.start and range.end?
> > 
> > After calling adjust_range_if_pmd_sharing_possible, range.start - range.end
> > could be much greater than the range we actually want to unmap.  The range
> > gets adjusted to account for pmd sharing if that is POSSIBLE.  It does not
> > know for sure if we will actually 'unshare a pmd'.
> > 
> > I suppose adjust_range_if_pmd_sharing_possible could be modified to actually
> > check if unmapping will result in unsharing, but it does not do that today.
> 
> Thanks for the explanation. It’s probably me, but I am still not sure that I
> understand the the different between __unmap_hugepage_range() using (start,
> end) and __zap_page_range_single() using (address, range.end). Perhaps it
> worth a comment in the code?

__zap_page_range_single is wrong.  It should have been updated to use
the range address - (address + size).

At Peter's suggestion the mmu notifier updates will be broken out in a
separate patch.  I will also add comments, to make this easier to follow.

> But anyhow… shouldn’t unmap_hugepage_range() call
> mmu_notifier_invalidate_range_start()?

Yes, thanks!

-- 
Mike Kravetz