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, 1 Sep 2022 13:29:19 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Sidhartha Kumar <sidhartha.kumar@oracle.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, songmuchun@bytedance.com,
        vbabka@suse.cz, william.kucharski@oracle.com, dhowells@redhat.com,
        peterx@redhat.com, arnd@arndb.de, ccross@google.com,
        hughd@google.com, ebiederm@xmission.com
Subject: Re: [PATCH 2/7] mm: add private field of first tail to struct page
 and struct folio
Message-ID: <YxEWH9P26bzuSK1U@monkey>
References: <20220829230014.384722-1-sidhartha.kumar@oracle.com>
 <20220829230014.384722-3-sidhartha.kumar@oracle.com>
 <YxDsu8Ol/yOg7sMV@monkey>
 <YxD6215MS+L+tGLc@casper.infradead.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YxD6215MS+L+tGLc@casper.infradead.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?mzak+CNYokVi7ppNWY3S++HLCiOo8isvYfnLt5zJV92MJrQAKpEc4xCX6GU8?=
 =?us-ascii?Q?EzIQKArilgT+fHMoNWoNx0qpG0x+KAP0O7JUafTvma56Iyy6UHYYsnFI0Hq6?=
 =?us-ascii?Q?HAgyLvSARLhh2r2bdSHdbk06qFX1zsS6rb850TCcHHFawf9AYrFawtLJ2Biw?=
 =?us-ascii?Q?RLxGZOAMoUfERaJVKVge/3RfW4nIcES5z3x/zCfJVm+oOfusOULnMftUfvA+?=
 =?us-ascii?Q?aux7nvQDXqZ5LRX4F+c0s6A7HPZzSp/k78BI07Z9wCPXis0+aA8LmXPdzsJV?=
 =?us-ascii?Q?nH3mLtrGwV+ba6Hyc2SJk8qWKRYW2EcFotOVojQ5OOjSLoxeefeeMsA1rzQx?=
 =?us-ascii?Q?+Zwjaq2cbU9F7ggciH0WYc3CyMITBu2VPeFXbIzthNdl59iWjtijvmJSXapr?=
 =?us-ascii?Q?s8ofA50/Exkzw26ssh8G/75Q1hyKMTMwxg+LyIh7e/p+b/nDf/p92slYUOuh?=
 =?us-ascii?Q?LEZFEc0yYCp8B258VI6Z2iKbXm4mCa3y7C8/ywLHDlIW6PkqRL+Lu5hUfsWl?=
 =?us-ascii?Q?JZVG6kn4ogavf14QGZSrVEM8RkZgybzoZqnzMQlrvsSmwwgoWNILGp1C0JYV?=
 =?us-ascii?Q?fmQNcbkjTgoMvRpkyqB4hUwUWIASknzztKTnxGd4LCF/GOluZub21CwLUgIE?=
 =?us-ascii?Q?WWK+VcEQfuEq0Vimci7bwtcScTvYmyAziKMMk5NMAQeOEZ3/5u3NbSKaVFV9?=
 =?us-ascii?Q?VZ6aZsm48CIMnKpL3MQq2oBcNw5QHItaW/JRuzMFLEXC+Ne0JvhBSxxd7iNr?=
 =?us-ascii?Q?R/1/qfRn+4xbGTKnGmmM1lQmpiB9dfaYrlPwaXEnMKbzXatpP9NjGiqaUVDq?=
 =?us-ascii?Q?mQfIAD+zh5+12XQlBJnDSDevFqPISOme64qrouslWd2cBCDeumEv2c5XwFEh?=
 =?us-ascii?Q?/m8sS7b/zmx7bXFXIAGkvVfV4jFh6GpylJfGtscsICSUszbxA16ACE9rUFze?=
 =?us-ascii?Q?moDVhlJ6wPvQlI3vampMJVqwStIyqadoftrnI5kK2+HMU3coRD+AfEL1nc6T?=
 =?us-ascii?Q?Ftl3ClpCIBljZBRVZTBUxFJyyPm0NdWyuaOau4JUc8MiToKrDU5PE/H09fKR?=
 =?us-ascii?Q?dPdoqZwCsFxVEGm3dyp6IjGopCujl/InxiKfE8GcKDBH251lk7Ni1su0Wret?=
 =?us-ascii?Q?G+kL7fl38Aj1Vjx6xbe/CoYX7XEXniK2XG9nME0gA2erpDzk1BtNH5zBkQWo?=
 =?us-ascii?Q?schr7+/TqIMs9xEPlpsegUFuOjovNy0rUdUmNHmEKeeUBi6ocg9FpAd4GZJK?=
 =?us-ascii?Q?frsRaiybmRAx+eki6a36j+JhT/UP6aC66HQ6E7AzDIdWc5TSLEsBFoJ1CXld?=
 =?us-ascii?Q?nh+FqLw5de4PLJ2j7/WkXYJuxnaeqlo0jH4Lkej1Knx2VVN8id27J+6ZgEgn?=
 =?us-ascii?Q?dZZVV86tGuzES0vKHHbsOrpxmsEN+f5s/jJXFN44zIeRIPy2bYY/xV5mTgOR?=
 =?us-ascii?Q?5dUhY63PP338FdJ/rHxdImZBETGXKtDYZwxqikrkNCxisX3FVtTrjJY1Ot/y?=
 =?us-ascii?Q?yEYKigOg3wE36Amp+TtfEXKaSrhgdliy04PnhaDfEoFn+jKHEjoBPFnHN9K4?=
 =?us-ascii?Q?343VizaNiRbB4XUfsdAlDa/Mnn3t7DFH4i9Gol934XhS0GPIfPnraXP4W/ci?=
 =?us-ascii?Q?Wg=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 36fee558-dc32-457b-9ad4-08da8c58a79f
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Sep 2022 20:29:23.4873
 (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: J/rETroie5TKqAmzER/sfmu59aZhWmKaijvv22oT9vnX0Orkszglz/0q8/qdUjjDhUddSmFgyg+AioJ1S6SGOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5577
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-09-01_12,2022-08-31_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999
 suspectscore=0 adultscore=0 bulkscore=0 mlxscore=0 spamscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2207270000 definitions=main-2209010088
X-Proofpoint-GUID: FyClZldPnqK3b-e-w98T0lbRSDmjgNyi
X-Proofpoint-ORIG-GUID: FyClZldPnqK3b-e-w98T0lbRSDmjgNyi
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,T_SCC_BODY_TEXT_LINE
        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 09/01/22 19:32, Matthew Wilcox wrote:
> On Thu, Sep 01, 2022 at 10:32:43AM -0700, Mike Kravetz wrote:
> > Not really an issue with this patch, but it made me read more of this
> > comment about folios.  It goes on to say ...
> > 
> >  * same power-of-two.  It is at least as large as %PAGE_SIZE.  If it is
> >  * in the page cache, it is at a file offset which is a multiple of that
> >  * power-of-two.  It may be mapped into userspace at an address which is
> >  * at an arbitrary page offset, but its kernel virtual address is aligned
> >  * to its size.
> >  */
> > 
> > This series is to begin converting hugetlb code to folios.  Just want to
> > note that 'hugetlb folios' have specific user space alignment restrictions.
> > So, I do not think the comment about arbitrary page offset would apply to
> > hugetlb.
> > 
> > Matthew, should we note that hugetlb is special in the comment?  Or, is it
> > not worth updating?
> 
> I'm open to updating it if we can find good wording.  What I'm trying
> to get across there is that when dealing with folios, you can assume
> that they're naturally aligned physically, logically (in the file) and
> virtually (kernel address), but not necessarily virtually (user
> address).  Hugetlb folios are special in that they are guaranteed to
> be virtually aligned in user space, but I don't know if here is the
> right place to document that.  It's an additional restriction, so code
> which handles generic folios doesn't need to know it.

Fair enough.  No need to change.  It just caught my eye.

> > Also, folio_get_private_1 will be used for the hugetlb subpool pointer
> > which resides in page[1].private.  This is used in the next patch of
> > this series.  I'm sure you are aware that hugetlb also uses page private
> > in sub pages 2 and 3.  Can/will/should this method of accessing private
> > in sub pages be expanded to cover these as well?  Expansion can happen
> > later, but if this can not be expanded perhaps we should come up with
> > another scheme.
> 
> There's a few ways of tackling this.  What I'm currently thinking is
> that we change how hugetlbfs uses struct page to store its extra data.
> It would end up looking something like this (in struct page):
> 
> +++ b/include/linux/mm_types.h
> @@ -147,9 +147,10 @@ struct page {
>                 };
>                 struct {        /* Second tail page of compound page */
>                         unsigned long _compound_pad_1;  /* compound_head */
> -                       unsigned long _compound_pad_2;
>                         /* For both global and memcg */
>                         struct list_head deferred_list;
> +                       unsigned long hugetlbfs_private_2;
> +                       unsigned long hugetlbfs_private_3;
>                 };
>                 struct {        /* Page table pages */
>                         unsigned long _pt_pad_1;        /* compound_head */
> 
> although we could use better names and/or types?  I haven't looked to
> see what you're storing here yet.  And then we can make the
> corresponding change to struct folio to add these elements at the
> right place.

I am terrible at names.  hugetlb is storing pointers in the private fields.
FWICT, something like this would work.

> 
> Does that sound sensible?

-- 
Mike Kravetz