On mercoledì 23 novembre 2022 00:24:26 CET Fabio M. De Francesco wrote:
> On lunedì 21 novembre 2022 19:53:54 CET Jason Gunthorpe wrote:
> > On Wed, Nov 16, 2022 at 01:37:14PM +0100, Fabio M. De Francesco wrote:
> > > > - return (void *)(uintptr_t)mr->map[m]->buf[n].addr + offset;
> > > > + if (mr->ibmr.type == IB_MR_TYPE_USER) {
> > > > + char *paddr;
> > > > + struct page *pg = mr->map[m]->buf[n].page;
> > > > +
> > > > + paddr = kmap_local_page(pg);
> > > > + if (paddr == NULL) {
> > > > + pr_warn("Failed to map page");
> > > > + return NULL;
> > > > + }
> > >
> > > I know nothing about this code but I am here as a result of regular
checks
> > > for changes to HIGHMEM mappings across the entire kernel. So please
>
> forgive
>
> > > me if I'm objecting to the correct changes.
> > >
> > > 1) It looks like this code had a call to page_address() and you
converted
>
> it
>
> > > to mapping with kmap_local_page().
> >
> > It only worked properly because the kconfig is blocking CONFIG_HIGHMEM
> > so ZONE_HIGHMEM doesn't exist. These pages are obtained from GUP and
> > could be anything.
> >
> > This is done indirectly through
> >
> > config INFINIBAND_VIRT_DMA
> >
> > def_bool !HIGHMEM
> >
> > But this patch is undoing parts of what are causing that restriction
> > by using the proper APIs. Though I'm not sure if we can eventually get
> > to remove it for RXE completely.
> > Jason
>
> Ah, OK. This code was for !HIGHMEM kernels...
>
> I understand but, FWIW I doubt that the use of page_address(), instead of
> kmapping, should ever be made on the bases that kconfig is blocking HIGHMEM.
>
> However, if I understand correctly, that restriction (!HIGHMEM) is going to
be
> removed. Therefore, page_address() will break on HIGHMEM and Jason is
> correctly converting to kmap_local_page().
Jason, I'm sorry for the erroneous attribution :-(
Fabio
> There is only one thing left... I think that he missed another mail from me.
> The one you responded to was missing the final paragraph, so I sent another
> message few minutes later.
>
> kmap_local_page() returns always valid pointers to kernel virtual addresses.
I
> can't see any ways for kmap_local_page() to return NULL. Therefore, I was
> asking for the reason to check for NULL.
>
> Thanks,
>
> Fabio
