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;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     Jason Gunthorpe <jgg@ziepe.ca>
Cc:     Li Zhijian <lizhijian@fujitsu.com>,
        Zhu Yanjun <zyjzyj2000@gmail.com>,
        Leon Romanovsky <leon@kernel.org>, linux-rdma@vger.kernel.org,
        linux-kernel@vger.kernel.org, Ira Weiny <ira.weiny@intel.com>
Subject: Re: [for-next PATCH 4/5] RDMA/rxe: refactor iova_to_vaddr
Date:   Wed, 23 Nov 2022 00:55:16 +0100
Message-ID: <4786191.31r3eYUQgx@suse>
In-Reply-To: <2158152.NgBsaNRSFp@suse>
References: <1668141030-2-1-git-send-email-lizhijian@fujitsu.com> <Y3vJQkc40o5zfM5v@ziepe.ca> <2158152.NgBsaNRSFp@suse>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
Precedence: bulk

On mercoled=EC 23 novembre 2022 00:24:26 CET Fabio M. De Francesco wrote:
> On luned=EC 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 =3D=3D IB_MR_TYPE_USER) {
> > > > +             char *paddr;
> > > > +             struct page *pg =3D mr->map[m]->buf[n].page;
> > > > +
> > > > +             paddr =3D kmap_local_page(pg);
> > > > +             if (paddr =3D=3D NULL) {
> > > > +                     pr_warn("Failed to map page");
> > > > +                     return NULL;
> > > > +             }
> > >=20
> > > I know nothing about this code but I am here as a result of regular=20
checks
> > > for changes to HIGHMEM mappings across the entire kernel. So please
>=20
> forgive
>=20
> > > me if I'm objecting to the correct changes.
> > >=20
> > > 1) It looks like this code had a call to page_address() and you=20
converted
>=20
> it
>=20
> > > to mapping with kmap_local_page().
> >=20
> > 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.
> >=20
> > This is done indirectly through
> >=20
> > config INFINIBAND_VIRT_DMA
> >=20
> >         def_bool !HIGHMEM
> >=20
> > 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
>=20
> Ah, OK. This code was for !HIGHMEM kernels...
>=20
> 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 HIGHM=
EM.
>=20
> However, if I understand correctly, that restriction (!HIGHMEM) is going =
to=20
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 :-(

=46abio
=20
> 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 anoth=
er
> message few minutes later.
>=20
> kmap_local_page() returns always valid pointers to kernel virtual address=
es.=20
I
> can't see any ways for kmap_local_page() to return NULL. Therefore, I was
> asking for the reason to check for NULL.
>=20
> Thanks,
>=20
> Fabio