linux-kernel - Re: [RFC PATCH rdma-next 01/10] RDMA: mr: Introduce is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Jan 2022 06:12:35 +0000
From:   "lizhijian@...itsu.com" <lizhijian@...itsu.com>
To:     Jason Gunthorpe <jgg@...pe.ca>,
        "lizhijian@...itsu.com" <lizhijian@...itsu.com>
CC:     "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "zyjzyj2000@...il.com" <zyjzyj2000@...il.com>,
        "aharonl@...dia.com" <aharonl@...dia.com>,
        "leon@...nel.org" <leon@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "mbloch@...dia.com" <mbloch@...dia.com>,
        "liangwenpeng@...wei.com" <liangwenpeng@...wei.com>,
        "yangx.jy@...itsu.com" <yangx.jy@...itsu.com>,
        "rpearsonhpe@...il.com" <rpearsonhpe@...il.com>,
        "y-goto@...itsu.com" <y-goto@...itsu.com>,
        "dan.j.williams@...el.com" <dan.j.williams@...el.com>
Subject: Re: [RFC PATCH rdma-next 01/10] RDMA: mr: Introduce is_pmem


Add Dan to the party :)

May i know whether there is any existing APIs to check whether
a va/page backs to a nvdimm/pmem ?



On 06/01/2022 08:21, Jason Gunthorpe wrote:
> On Tue, Dec 28, 2021 at 04:07:08PM +0800, Li Zhijian wrote:
>> We can use it to indicate whether the registering mr is associated with
>> a pmem/nvdimm or not.
>>
>> Currently, we only assign it in rxe driver, for other device/drivers,
>> they should implement it if needed.
>>
>> RDMA FLUSH will support the persistence feature for a pmem/nvdimm.
>>
>> Signed-off-by: Li Zhijian <lizhijian@...fujitsu.com>
>>   drivers/infiniband/sw/rxe/rxe_mr.c | 47 ++++++++++++++++++++++++++++++
>>   include/rdma/ib_verbs.h            |  1 +
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
>> index 7c4cd19a9db2..bcd5e7afa475 100644
>> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
>> @@ -162,6 +162,50 @@ void rxe_mr_init_dma(struct rxe_pd *pd, int access, struct rxe_mr *mr)
>>   	mr->type = IB_MR_TYPE_DMA;
>>   }
>>   
>> +// XXX: the logic is similar with mm/memory-failure.c
>> +static bool page_in_dev_pagemap(struct page *page)
>> +{
>> +	unsigned long pfn;
>> +	struct page *p;
>> +	struct dev_pagemap *pgmap = NULL;
>> +
>> +	pfn = page_to_pfn(page);
>> +	if (!pfn) {
>> +		pr_err("no such pfn for page %p\n", page);
>> +		return false;
>> +	}
>> +
>> +	p = pfn_to_online_page(pfn);
>> +	if (!p) {
>> +		if (pfn_valid(pfn)) {
>> +			pgmap = get_dev_pagemap(pfn, NULL);
>> +			if (pgmap)
>> +				put_dev_pagemap(pgmap);
>> +		}
>> +	}
>> +
>> +	return !!pgmap;
> You need to get Dan to check this out, but I'm pretty sure this should
> be more like this:
>
> if (is_zone_device_page(page) && page->pgmap->type == MEMORY_DEVICE_FS_DAX)

Great, i have added him.



>
>
>> +static bool iova_in_pmem(struct rxe_mr *mr, u64 iova, int length)
>> +{
>> +	struct page *page = NULL;
>> +	char *vaddr = iova_to_vaddr(mr, iova, length);
>> +
>> +	if (!vaddr) {
>> +		pr_err("not a valid iova %llu\n", iova);
>> +		return false;
>> +	}
>> +
>> +	page = virt_to_page(vaddr);
> And obviously this isn't uniform for the entire umem, so I don't even
> know what this is supposed to mean.

My intention is to check if a memory region belongs to a nvdimm/pmem.
The approach is like that:
iova(user space)-+                     +-> page -> page_in_dev_pagemap()
                  |                     |
                  +-> va(kernel space) -+
Since current MR's va is associated with map_set where it record the relations
between iova and va and page. Do do you mean we should travel map_set to
get its page ? or by any other ways.

  





>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>> index 6e9ad656ecb7..822ebb3425dc 100644
>> +++ b/include/rdma/ib_verbs.h
>> @@ -1807,6 +1807,7 @@ struct ib_mr {
>>   	unsigned int	   page_size;
>>   	enum ib_mr_type	   type;
>>   	bool		   need_inval;
>> +	bool		   is_pmem;
> Or why it is being stored in the global struct?

Indeed, it's not strong necessary. but i think is_pmem should belongs to a ib_mr
so that it can be checked by other general code when they needed even though no
one does such checking so far.


Thanks
Zhijian



>
> Jason