Re: [PATCH V10 33/44] kmap: Make kmap work for devmap protected pages

From: Ira Weiny <ira.weiny@intel.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Rick Edgecombe <rick.p.edgecombe@intel.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH V10 33/44] kmap: Make kmap work for devmap protected pages
Date: Thu, 19 May 2022 13:29:41 -0700	[thread overview]
Message-ID: <YoaotWPR0H5Mykff@iweiny-desk3> (raw)
In-Reply-To: <YoShWMGVj9ZJbWqT@infradead.org>

On Wed, May 18, 2022 at 12:33:44AM -0700, Christoph Hellwig wrote:
> On Tue, May 17, 2022 at 03:46:55PM -0700, Ira Weiny wrote:
> > Ok there is a trade off here which I'm not sure is a good idea.
> > 
> > In order to make kmap faster I carefully placed the devmap_protected() call
> > completely inline.[1]  This specifically checks the PGMAP_PROTECTION flag in
> > dev_pagemap.[1]
> > 
> > I see only 2 ways of not including memremap.h in highmem-internal.h.
> > 
> > 1) Make this check a function call and place it in memremap.c
> > 2) Move struct dev_pagemap (and it's dependencies) to another header.
> > 
> > Number 2 negates any positive effect of splitting the header file.
> > 
> > Number 1 is going to force some overhead on all ZONE_DEVICE pages if PMEM is in
> > the system.
> > 
> > Do you think the extra run time overhead is a reasonable trade off to
> > eliminating the header?
> 
> Well, given how big these devmap helpes are it seems like they should be
> out of line anyway due to the code size impact.  It would be great to
> compare the code size for the cases of:

Fair enough.  I did a few experiments with all yes config modified only to
remove LOCKDEP because those numbers were a bit skewed IMO.

> 
>  1) baseline kernel

Text		Data		bss
293450036	337712849	19907196

>  2) devmap protection inline

This is V10

Text		Data		bss
293461422	337759561	19882620

From Baseline:
 + 11386	+ 46712
   0.0038%	0.014%

2.5) devmap partly inline (This cleans up the headers by adding a new header
  for devmap protection and not using memremap.h in highmem-internal.h)

Text		Data		bss
293460722	337759593	19882620

From Baseline:
 + 10686	+ 46744
  0.0036%	0.014%

>  3) devmap protection out of line

Text		Data		bss
293458303	337734401	19890812

From Baseline:
 + 8267		+ 21552
  0.0028%	0.0064%

From these numbers it seems reasonable to have some of devmap_protected() out
of line to clean up the header dependencies.

Thanks,
Ira