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;
IronPort-SDR: +OabCUpeHLuEIHxaTTb4qaSPrR9tZoejP7VtW66YTjqlgOWTOZo5PPkjrLFvDlxNSTbqF4go/L
 SNhBRSx38/qPSvxs4KGhimM41pTloyTW3MNsWF/pBAYqsRgfCWeG2MZLl1YVSVePlcsOJBEme3
 CjQTqaJMZHHgqxWjM8sufDRSvullH8G1xbtSnd2al/PZpwgx8ewGmmUhTNTkJBz4JozglUzTxE
 DmutOaF4H6eOr5J8/iSLgO8htTwSu7RM1VFjJwtT577nPElwweSb+drDkG841SyYxmZvNhdKtZ
 V1g=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     "ira.weiny@intel.com" <ira.weiny@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        David Sterba <dsterba@suse.cz>
CC:     Boris Pismenny <borisp@mellanox.com>,
        Or Gerlitz <gerlitz.or@gmail.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Matthew Wilcox <willy@infradead.org>,
        "hch@infradead.org" <hch@infradead.org>,
        Dan Williams <dan.j.williams@intel.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Eric Biggers <ebiggers@kernel.org>, "clm@fb.com" <clm@fb.com>,
        "josef@toxicpanda.com" <josef@toxicpanda.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH V2 1/8] mm/highmem: Lift memcpy_[to|from]_page to core
Thread-Topic: [PATCH V2 1/8] mm/highmem: Lift memcpy_[to|from]_page to core
Thread-Index: AQHW/3WAdYNjj6ILr021d+mWvVgb4g==
Date:   Wed, 10 Feb 2021 06:33:24 +0000
Message-ID: <BYAPR04MB49655E6A3BD935B15DCC468B868D9@BYAPR04MB4965.namprd04.prod.outlook.com>
References: <20210210062221.3023586-1-ira.weiny@intel.com>
 <20210210062221.3023586-2-ira.weiny@intel.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: fb1ee856-1f03-4c30-27ff-08d8cd8dc4bc
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Feb 2021 06:33:24.6274
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: R2EPrzhQXpsQrP6C41uBXaeraAtYQlaCVprN7zfL9Xt4k9tVJ6xDeYPAHVxxnuUX9kjU3abUKhLK6CZoQujmWL9k0AtptnFSUDveLdNarvo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB4039
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2/9/21 22:25, ira.weiny@intel.com wrote:=0A=
> From: Ira Weiny <ira.weiny@intel.com>=0A=
>=0A=
> Working through a conversion to a call kmap_local_page() instead of=0A=
> kmap() revealed many places where the pattern kmap/memcpy/kunmap=0A=
> occurred.=0A=
>=0A=
> Eric Biggers, Matthew Wilcox, Christoph Hellwig, Dan Williams, and Al=0A=
> Viro all suggested putting this code into helper functions.  Al Viro=0A=
> further pointed out that these functions already existed in the iov_iter=
=0A=
> code.[1]=0A=
>=0A=
> Various locations for the lifted functions were considered.=0A=
>=0A=
> Headers like mm.h or string.h seem ok but don't really portray the=0A=
> functionality well.  pagemap.h made some sense but is for page cache=0A=
> functionality.[2]=0A=
>=0A=
> Another alternative would be to create a new header for the promoted=0A=
> memcpy functions, but it masks the fact that these are designed to copy=
=0A=
> to/from pages using the kernel direct mappings and complicates matters=0A=
> with a new header.=0A=
>=0A=
> Placing these functions in 'highmem.h' is suboptimal especially with the=
=0A=
> changes being proposed in the functionality of kmap.  From a caller=0A=
> perspective including/using 'highmem.h' implies that the functions=0A=
> defined in that header are only required when highmem is in use which is=
=0A=
> increasingly not the case with modern processors.  However, highmem.h is=
=0A=
> where all the current functions like this reside (zero_user(),=0A=
> clear_highpage(), clear_user_highpage(), copy_user_highpage(), and=0A=
> copy_highpage()).  So it makes the most sense even though it is=0A=
> distasteful for some.[3]=0A=
>=0A=
> Lift memcpy_to_page() and memcpy_from_page() to pagemap.h.=0A=
>=0A=
> [1] https://lore.kernel.org/lkml/20201013200149.GI3576660@ZenIV.linux.org=
.uk/=0A=
>     https://lore.kernel.org/lkml/20201013112544.GA5249@infradead.org/=0A=
>=0A=
> [2] https://lore.kernel.org/lkml/20201208122316.GH7338@casper.infradead.o=
rg/=0A=
>=0A=
> [3] https://lore.kernel.org/lkml/20201013200149.GI3576660@ZenIV.linux.org=
.uk/#t=0A=
>     https://lore.kernel.org/lkml/20201208163814.GN1563847@iweiny-DESK2.sc=
.intel.com/=0A=
>=0A=
> Cc: Boris Pismenny <borisp@mellanox.com>=0A=
> Cc: Or Gerlitz <gerlitz.or@gmail.com>=0A=
> Cc: Dave Hansen <dave.hansen@intel.com>=0A=
> Suggested-by: Matthew Wilcox <willy@infradead.org>=0A=
> Suggested-by: Christoph Hellwig <hch@infradead.org>=0A=
> Suggested-by: Dan Williams <dan.j.williams@intel.com>=0A=
> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>=0A=
> Suggested-by: Eric Biggers <ebiggers@kernel.org>=0A=
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>=0A=
=0A=
Thanks for adding a new line in the new calls after variable declaration.=
=0A=
Looks good.=0A=
=0A=
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>=0A=
=0A=