On Fri, Jan 21, 2022 at 08:58:37AM -0400, Jason Gunthorpe wrote: > On Thu, Jan 20, 2022 at 08:07:36PM +0800, Li, Zhijian wrote: > > > diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c > > b/drivers/infiniband/sw/rxe/rxe_mr.c > > index 0621d387ccba..978fdd23665c 100644 > > +++ b/drivers/infiniband/sw/rxe/rxe_mr.c > > @@ -260,7 +260,8 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 > > length, u64 iova, > > num_buf = 0; > > } > > > > - vaddr = page_address(sg_page_iter_page(&sg_iter)); > > + // FIXME: don't forget to kunmap_local(vaddr) > > + vaddr = kmap_local_page(sg_page_iter_page(&sg_iter)); > > No, you can't leave the kmap open for a long time. The kmap has to > just be around the usage. Indeed Jason is correct here. A couple of details here. First kmap_local_page() is only valid within the current thread of execution. So what you propose above will not work at all. Second, kmap() is to be avoided. Finally, that page_address() should be avoided IMO and will be broken, at least for persistent memory pages, once some of my work lands.[*] Jason would know better, but I think page_address should be avoided in all driver code. But there is no clear documentation on that. Taking a quick look at rxe_mr.c buf->addr is only used in rxe_mr_init_user(). You need to kmap_local_page() around that access. What else is struct rxe_phys_buf->addr used for? Can you just map the page when you need it in rxe_mr_init_user()? If you must create a mapping that is permanent you could look at vmap(). Ira > > Jason
