From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E806E7717D for ; Wed, 11 Dec 2024 04:37:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BAD9F8D0015; Tue, 10 Dec 2024 23:37:48 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B5D628D0013; Tue, 10 Dec 2024 23:37:48 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A25328D0015; Tue, 10 Dec 2024 23:37:48 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 85B648D0013 for ; Tue, 10 Dec 2024 23:37:48 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 39E28C1258 for ; Wed, 11 Dec 2024 04:37:48 +0000 (UTC) X-FDA: 82881419754.11.8A5793A Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf27.hostedemail.com (Postfix) with ESMTP id 99ABF40002 for ; Wed, 11 Dec 2024 04:37:20 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=liE4Cl1P; spf=none (imf27.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733891849; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=fafpTVDXQOsJRxZ5gDh32Cs9vqOM9tXISh/UQeDMyrc=; b=dLJNTZU4dhamFsv1nuCdbvqLbD/9pevA+Z17rz4TVR2IzzlIjeCbjrXhl4ZWzuMJVr9GIN BZQhFO2slJJ/Aikl4daE2r7iwcZZwKajEr9ss1oqNp4LL85Wwv4gV7Giu05dl5mycYZnzS c/Pa2nOpGqUi2xjMIHomYRDY7HJ6ac8= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=liE4Cl1P; spf=none (imf27.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733891849; a=rsa-sha256; cv=none; b=FzY6UkF6r4o9Z9LVmZUrtOjU6Sv9kcH8H7gQFbBDhzo+Cvn/zymI25p8ZHHg1JU69lNVPL 6NkNgrt+OMIRUgDbQwM7HvB/r/p3Fz4SRdw5mt95E9Y8QDEbJklZJQ2IuiUPkZxjomYxiz vlfCS7iQxDpsPR+oVREs3qXm0eqYOvU= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=fafpTVDXQOsJRxZ5gDh32Cs9vqOM9tXISh/UQeDMyrc=; b=liE4Cl1Pv56fLfJv3hp3dCYKWY SVeiGZUyX0kFGJ1Q5k+qyPb517wIfz0NdXBc/HOikPt6RMDWJTW1LoTPiIYs4ZAV/2cFF6fA/OJnx HMhYlt5uAsQsr862o9qYvLG5jd2Z3dD+dc/N+7SROitqBUjUCoEnQARJ0tqI9Jv2gs9Bv1WHF08p5 AzuZ0lS8/soTsvhJWRkLxjMBEMBooL29bGOCYUj70r0T2P1NSXYInF96rfAe9PMmk1a1Yzri6EnRz oKbPmcU2KfNW9cCwC8dpPoD0rW/Liz25Fxg4JSluqUGyVb8PiOWqFHFph1FgXkAhCH/KVurRw9MMv YVLuYpbA==; Received: from willy by casper.infradead.org with local (Exim 4.98 #2 (Red Hat Linux)) id 1tLETo-0000000DrGM-1Jn5; Wed, 11 Dec 2024 04:37:44 +0000 Date: Wed, 11 Dec 2024 04:37:44 +0000 From: Matthew Wilcox To: Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: Minchan Kim , Sergey Senozhatsky , Alex Shi , linux-mm@kvack.org Subject: Re: [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes Message-ID: References: <20241205175000.3187069-1-willy@infradead.org> <20241205175000.3187069-22-willy@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 99ABF40002 X-Rspamd-Server: rspam12 X-Stat-Signature: owbycc6wzdnwoqydk7x3mweedh3s4ar1 X-Rspam-User: X-HE-Tag: 1733891840-717285 X-HE-Meta: U2FsdGVkX18wWiEkP8ysBtmPekTMV+l2uAe1o8UROtcLhled97YeXRQMEdwlCfZlQFTz9d2w7EG1T8YIeapXDg1dYMPqGC1OI+/aexk5mMRooNnVZjcqqMKS/4DlqjUYNCnG/HtGbh470Umx2ebCw5/kFHlyJRfV1aCmCKMmG8EyfxL3bZcjtl94DF/EHciUeRnseUeHUy4QNkGOmiqlZGgaRHYHE3c0uX28MVqZdwxn5q4HJ1EJsE7yLeTglE9OmVNIb1dH4/G38MDxmDrd2S/lAJuEWVl9fI3OrKWmotLzhmaZBHlE17fRRNBNwRAWKW84f1VTGUJW2X42fh4y1fQmSeT8YjHUIZIyg54Z/Og3l5oWdiC8MVyTiq1NiK140/wargK2XuEieSdnnchj/j8v4p81VCM6s4g1VGjIUh3WSWOt7KJb3KP+HZhvNhhMpfvDD6igaz+SrEnBetBVCOfJzBFmbaeDY6/bprdnDAsDoVJDKdztMqZoi9pvBRvbfXdMbqIuq4tp8+W/Ey/Wz8x1HbUpaA4zB+vO5TNVMmD+H4yWR3KHh54pPxyGrPbszhy6irbb5VJpGtwISEif45l8ZCBJAuVZWBPItyXRgallCL7rVhWl5qxOeJpXJtc2MzSEzurmdnkjrTQfaZMeiSuY2eb2r+IjCVP1Y8JqlMl9vXLZFtZX3EN+bk7b1b1EYUXHSNOAPFuciB9CLbZxYhPprMy5LIgD1d4QSltI7YCLhxPsAsaqDKkI11xFVtwINAr73Zl5GoeD6bQaXUUfKQKqWQeLuZ6vU5YIi8ycw/0Oti+s6ol/cPBAQVpicK6AwEtQZ3wU8zmk8iXEqF+jrkqhnOzKfr2+rcOJGU5daSG8lLUuLQ3ChF8+7d4EDcVdVAdBRmXL+AfYkjRvF58/6SEUzhgTOR3KEITuI8sIK5/IyjNZYrDPadJpspQpnSaksqEsMJ+uHzHrnDZy+Vu wS3beslm M2GH9djmxj7aQaGYWzhGtQuW/t68A5x6EWCGKGqkMeShOe3RoWqy0s9+/hJ4SwXjMqvIGR3yUbnkffMO/wZvbQweeJiSxWU0GGNAvQXRIzaxDejHM23Q0AmPMF72eDpxQcna0vhKHZFmDbn8CHTAWKxMc1vtqzJpWSjAP256xBy3+HEpqjrd/i5LXWdSVqZi34p5ZqX0wZWweZlCc7AGdrxVrs5hVVWMflcQdLmL7ZZ4Q78RW8so9Ms7QfJoVir5R/Ul28gIC5wC95f3qBDwk87tL3YnMBjRgHBNANETheU4p5zM= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Dec 11, 2024 at 12:46:43AM +0900, Hyeonggon Yoo wrote: > Talking about updating comments and code by replacing 'page' with 'zpdesc', > I'm not sure if it makes sense to replace all instances of 'page' with 'zpdesc'. > A zpdesc is a descriptor, not a page that contains data (at least that's > what I have been thinking while writing the initial patch series). Agreed. > In that context I'm still not sure if saying "a sub-zpdesc of a zspage", "lock zpdesc", > or "migrate zpdesc" makes sense because replacing 'page descriptor (aka. struct page)' with > 'zpool descriptor (aka. zpdesc)' doesn't mean zsmalloc is throwing away the conecept of pages. sub-zpdesc is a silly thing to say. subpage is equally silly. If it's a page, call it a page. However, locking the zpdesc does make sense -- we're locking the descriptor. > > /* > > * Following is how we use various fields and flags of underlying > > - * struct page(s) to form a zspage. > > + * struct zpdesc(page) to form a zspage. > > * > > - * Usage of struct page fields: > > - * page->private: points to zspage > > - * page->index: links together all component pages of a zspage > > + * Usage of struct zpdesc fields: > > + * zpdesc->zspage: points to zspage > > + * zpdesc->next: links together all component zpdescs of a zspage > > * For the huge page, this is always 0, so we use this field > > * to store handle. > > - * page->page_type: PGTY_zsmalloc, lower 24 bits locate the first object > > - * offset in a subpage of a zspage > > - * > > - * Usage of struct page flags: > > - * PG_private: identifies the first component page > > - * PG_owner_priv_1: identifies the huge component page > > + * zpdesc->first_obj_offset: PGTY_zsmalloc, lower 24 bits locate the first > > + * object offset in a subpage of a zspage > > * > > + * Usage of struct zpdesc(page) flags: > > + * PG_private: identifies the first component zpdesc > > + * PG_lock: lock all component zpdescs for a zspage free, serialize with > > */ > > I think this comment is unnecessary, as it's already documented in mm/zpdesc.h. > It can be removed in patch 01. Agreed. I did think about doing that, so if you want to do it too, that's two votes for doing it. > > @@ -1351,7 +1353,7 @@ static unsigned long obj_malloc(struct zs_pool *pool, > > /* record handle in the header of allocated chunk */ > > link->handle = handle | OBJ_ALLOCATED_TAG; > > else > > - /* record handle to page->index */ > > + /* record handle to zpdesc->handle */ > > zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG; > > the name of the field is already 'handle', > so we don't need a comment to explain it. Agreed. Do you want to take on producing v9 or do you want me to fold in your suggestions and send it?