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;
Message-ID: <3d88bbf3-8c55-46fc-731e-bc81e184b8d7@nvidia.com>
Date:   Mon, 5 Sep 2022 16:58:26 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.0
Subject: Re: [PATCH v9 3/8] block: add check when merging zone device pages
Content-Language: en-US
To:     Logan Gunthorpe <logang@deltatee.com>,
        linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
        linux-block@vger.kernel.org, linux-pci@vger.kernel.org,
        linux-mm@kvack.org
Cc:     Christoph Hellwig <hch@lst.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Dan Williams <dan.j.williams@intel.com>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        Don Dutile <ddutile@redhat.com>,
        Matthew Wilcox <willy@infradead.org>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Minturn Dave B <dave.b.minturn@intel.com>,
        Jason Ekstrand <jason@jlekstrand.net>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Xiong Jianxin <jianxin.xiong@intel.com>,
        Bjorn Helgaas <helgaas@kernel.org>,
        Ira Weiny <ira.weiny@intel.com>,
        Robin Murphy <robin.murphy@arm.com>,
        Martin Oliveira <martin.oliveira@eideticom.com>,
        Chaitanya Kulkarni <ckulkarnilinux@gmail.com>,
        Ralph Campbell <rcampbell@nvidia.com>,
        Stephen Bates <sbates@raithlin.com>
References: <20220825152425.6296-1-logang@deltatee.com>
 <20220825152425.6296-4-logang@deltatee.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20220825152425.6296-4-logang@deltatee.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NmNwdEhobEpXVmNHODVVRUpCVU1xbzRPWlhCZy9tbjA1UUg2b1hsS25jTHRG?=
 =?utf-8?B?Q0FlMGFHc3pTeFpBL1BybVlaK1NSNFEwNmRzL05xNzhXaG1kaHRJZkhmMkxm?=
 =?utf-8?B?N1N6alc5bHdubTZkaU9qMXVLWVZEaDhCYlZJUkFDL2xReDFLL1pnemtUeXVT?=
 =?utf-8?B?MGlxV3NON3Uwb0gydGVGZXdsZDJ0cHAyTStOUmhMTDgxdlFvalR1cVlTbURG?=
 =?utf-8?B?VFh3aFFISDFwK0VoWlBRYVltTndjUEZWTURjY3VtbnhmN2cyRGluR1RnMWVX?=
 =?utf-8?B?cmJ3eFZrVmh2eHhWU29ydkNNYUJXWDBraUJTZnNQUzBYbFM0MURCajg2T21L?=
 =?utf-8?B?VlRXVEtRcVBvQ0MwQUsycU02M0xsU1VMR09oUEN6YWtibjhkakl3Vy95cmpF?=
 =?utf-8?B?Y0NxRGl0NzVKWXNJaGlkeG9uVXNvWXlrZC9oRDd2VHFjdUlmdC8vZHF4cTlN?=
 =?utf-8?B?OURMOTBhcDh0VzE0dkc2YVZZaEZMZXRWbk1Hb1hUaStFVFUwVGM5czV6Q3cw?=
 =?utf-8?B?WkJSMjVYQk5aNUQ4TDFzSGt2NWJIMFVURVRmV24wV0ZvbUdlcFpRYVkycXkr?=
 =?utf-8?B?NlpubXB5RzE1MUpsQkxDSnVTYktvZVNHK3RhcjRSUGY2RkdrQXozd3pzd3ZN?=
 =?utf-8?B?NTFRTXNLKzE3SXFsbnJQWmVtUDJUd09EYkx3SGZzakxxYVo1Mm51NHpZc3hV?=
 =?utf-8?B?NDhuYWNva29HR0hzV1NSU1V0bWd5dDJxUllIL2xnY3ZyU0U0WDE0U2swclRo?=
 =?utf-8?B?dlY4SGsrRU50QmZXZy9VWThZRHhLclNrRXZiSEd3ODNHbkQ4bVQvaG1ZRDVm?=
 =?utf-8?B?aWpjZFJqOVJvakxEdFpUNjNaUEFhSmJuMjB6TnZVYmlwd0EzNTlqdGRnem9P?=
 =?utf-8?B?TnZsWlVOb0VBVVNLMHQ3c1FxSlpoNTh3NHFEa0JaMTlnQllHd1RlSFEyc25m?=
 =?utf-8?B?N3QwdEdxTTBKTjFSYkJTL29JLzZBTmY0eUdqZk5PZmoyY2lBZDN5VmdTUytU?=
 =?utf-8?B?OXhNeTRIcnBpcDFrZHNYOC9DMVJzNUludlkzOW1BWmMyYnFBVmgrT3J1Ynd2?=
 =?utf-8?B?V0dVeEJyZmJ5dmYzRWpjSHVpRmxxZVE3RkJKVytBam1tUFN2SEp1cUk1aDBM?=
 =?utf-8?B?VEhwYm5rQSs2N1QxcnRNSGIzcGNOZTJDaXJoOVdLOFZCd2V3bnpmbG95TE1I?=
 =?utf-8?B?QW5OT3A4RkFYaGZGeFNuZ24wZHhVcGRMeVJDT2c2aWttWGowZWMvdDZLTVNY?=
 =?utf-8?B?cS9qbmZRVGNpalVPLzBJY3p2ckhYM08veUpIT1BCUUpSc3Z4SlJ2MkRsbHFz?=
 =?utf-8?B?Q0doSGl4MVlSZGVDS0hQWlN3NWJEdnA3Z1RTNG5PL1B4aXF6OU1aMVR3TElh?=
 =?utf-8?B?MEQ3dVdJcUtiWWc1QjV6cVdiVk5XbGlNaDJhZStUeU1CNDZKUnM2azNadmJ0?=
 =?utf-8?B?SEN6anIyQnArTk0wWEQyUjljVnhFekduVXdFUFFYYmtndTRNbjVZMXAwVXRO?=
 =?utf-8?B?b1pPREg5ZGFaWWlJRmpzSXp4UjFveFR4dGV0RnNFWkMyMFl2MU93MU9OZlpq?=
 =?utf-8?B?SHE5alJGRmhrc2FIZ0hGaGtNNXdrTklsNklMTFBnOFQ4azhIZXJYTmxzWlAx?=
 =?utf-8?B?anpVZllCMS8rZFl0V2pBUGdkTEowZUxkK1dRbC9XcUFmQjhESGRQQkxGNzlI?=
 =?utf-8?B?UHgwZCtWUWx2V1RQSmptTlBheFAzckxSZTd1RFNlcE5YTmpheHBIMy9DRk1G?=
 =?utf-8?B?K3BiMnVqamYxY2twT3p0MUNuSUJ3dG9kUDlLeU1md09YT0pmV1BOL0RRR0hH?=
 =?utf-8?B?aUpjUEhmMnppWGwxeEMwVWFNMGpWd0tPNk9yVVhUM1R4V2x4SXZ3WEgyYnRt?=
 =?utf-8?B?RWJuaCtKd3VJQUkwZjltaTBoRDk4b05mRzYrNUs1M1Q1NFRrdG1WK29TeCt6?=
 =?utf-8?B?ampGWTUxcjlNdTVWM3E1a0N5KzE2N3pWa2NiRFl2cEJUWEVGR1YwREEwTmU2?=
 =?utf-8?B?Z0x5cWRPbTlJRTR3QmIwU09QeDNpNVhleGp3ZWpML3dGdnZ5MkM2dUtkOVBF?=
 =?utf-8?B?QmRwZTVydFZlWE5OL0M0R0VFVi91VkdmSlZ2QzMwbzBCQ3lsb1ZhWmhhWGRn?=
 =?utf-8?B?R2oxY3Z0bkxvWUlPRnBxUXFQK29zZEk3REZlNFozQmZwRlJRWHhBbzE5aUJn?=
 =?utf-8?B?eFE9PQ==?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4db04050-14db-4771-fed1-08da8f9a87ae
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Sep 2022 23:58:29.3132
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: K4XGaPkWYDysU7o7Camec6CdIi/irl+Vm3VdKnpAC44exqtPx2ke9t/io4yXb2enajnUElXEejhkN85WRThkqw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4242
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        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 8/25/22 08:24, Logan Gunthorpe wrote:
> Consecutive zone device pages should not be merged into the same sgl
> or bvec segment with other types of pages or if they belong to different
> pgmaps. Otherwise getting the pgmap of a given segment is not possible
> without scanning the entire segment. This helper returns true either if
> both pages are not zone device pages or both pages are zone device
> pages with the same pgmap.
> 
> Add a helper to determine if zone device pages are mergeable and use
> this helper in page_is_mergeable().
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  block/bio.c            |  2 ++
>  include/linux/mmzone.h | 24 ++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 3d3a2678fea2..969607bc1f4d 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -865,6 +865,8 @@ static inline bool page_is_mergeable(const struct bio_vec *bv,
>  		return false;
>  	if (xen_domain() && !xen_biovec_phys_mergeable(bv, page))
>  		return false;
> +	if (!zone_device_pages_have_same_pgmap(bv->bv_page, page))
> +		return false;
>  
>  	*same_page = ((vec_end_addr & PAGE_MASK) == page_addr);
>  	if (*same_page)
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index e24b40c52468..2c31915b057e 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -794,6 +794,25 @@ static inline bool is_zone_device_page(const struct page *page)
>  {
>  	return page_zonenum(page) == ZONE_DEVICE;
>  }
> +
> +/*
> + * Consecutive zone device pages should not be merged into the same sgl
> + * or bvec segment with other types of pages or if they belong to different
> + * pgmaps. Otherwise getting the pgmap of a given segment is not possible
> + * without scanning the entire segment. This helper returns true either if
> + * both pages are not zone device pages or both pages are zone device pages
> + * with the same pgmap.
> + */

Nice work on the documentation here. Approach looks correct.


Reviewed-by: John Hubbard <jhubbard@nvidia.com>

thanks,

-- 
John Hubbard
NVIDIA

> +static inline bool zone_device_pages_have_same_pgmap(const struct page *a,
> +						     const struct page *b)
> +{
> +	if (is_zone_device_page(a) != is_zone_device_page(b))
> +		return false;
> +	if (!is_zone_device_page(a))
> +		return true;
> +	return a->pgmap == b->pgmap;
> +}
> +
>  extern void memmap_init_zone_device(struct zone *, unsigned long,
>  				    unsigned long, struct dev_pagemap *);
>  #else
> @@ -801,6 +820,11 @@ static inline bool is_zone_device_page(const struct page *page)
>  {
>  	return false;
>  }
> +static inline bool zone_device_pages_have_same_pgmap(const struct page *a,
> +						     const struct page *b)
> +{
> +	return true;
> +}
>  #endif
>  
>  static inline bool folio_is_zone_device(const struct folio *folio)