Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Bart Van Assche <bvanassche@acm.org>
Cc: "Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com>,
	Yi Zhang <yi.zhang@redhat.com>,
	"Daisuke Matsuda (Fujitsu)" <matsuda-daisuke@fujitsu.com>,
	Zhu Yanjun <yanjun.zhu@intel.com>,
	Zhu Yanjun <yanjun.zhu@linux.dev>,
	"leon@kernel.org" <leon@kernel.org>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	"zyjzyj2000@gmail.com" <zyjzyj2000@gmail.com>,
	Bob Pearson <rpearsonhpe@gmail.com>
Subject: Re: [PATCH 1/1] RDMA/rxe: Fix blktests srp lead kernel panic with 64k page size
Date: Thu, 26 Oct 2023 10:43:00 -0300	[thread overview]
Message-ID: <20231026134300.GV691768@ziepe.ca> (raw)
In-Reply-To: <143f03b7-08ba-411c-a7ad-580141c06cfe@acm.org>

On Thu, Oct 26, 2023 at 06:28:37AM -0700, Bart Van Assche wrote:
> On 10/26/23 02:05, Zhijian Li (Fujitsu) wrote:
> > The root cause is that
> > 
> > rxe:rxe_set_page() gets wrong when mr.page_size != PAGE_SIZE where it only stores the *page to xarray.
> > So the offset will get lost.
> > 
> > For example,
> > store process:
> > page_size = 0x1000;
> > PAGE_SIZE = 0x10000;
> > va0 = 0xffff000020651000;
> > page_offset = 0 = va & (page_size - 1);
> > page = va_to_page(va);
> > xa_store(&mr->page_list, mr->nbuf, page, GFP_KERNEL);
> > 
> > load_process:
> > page = xa_load(&mr->page_list, index);
> > page_va = kmap_local_page(page) --> it must be a PAGE_SIZE align value, assume it as 0xffff000020650000
> > va1 = page_va + page_offset = 0xffff000020650000 + 0 = 0xffff000020650000;
> > 
> > Obviously, *va0 != va1*, page_offset get lost.
> > 
> > 
> > How to fix:
> > - revert 325a7eb85199 ("RDMA/rxe: Cleanup page variables in rxe_mr.c")
> > - don't allow ulp registering mr.page_size != PAGE_SIZE ?
> 
> Thank you Zhijian for this root-cause analysis.
> 
> Hardware RDMA adapters may not support the host page size (PAGE_SIZE)
> so disallowing mr.page_size != PAGE_SIZE would be wrong.

PAGE_SIZE must always be a valid way to construct a MR. We have code
in all the DMA drivers to break PAGE_SIZE chunks to something smaller
when building MRs.

rxe is missing this now with the xarray stuff since we can't encode
the struct page offset and a struct page in a single xarray entry. It
would have to go back to encoding pfns and doing pfn to page.

> If the rxe driver only supports mr.page_size == PAGE_SIZE, does this
> mean that RXE_PAGE_SIZE_CAP should be changed from
> 0xfffff000 into PAGE_SHIFT?

Yes

Jason

  reply	other threads:[~2023-10-26 13:43 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-13  1:18 [PATCH 1/1] RDMA/rxe: Fix blktests srp lead kernel panic with 64k page size Zhu Yanjun
2023-10-13 12:01 ` Daisuke Matsuda (Fujitsu)
2023-10-13 12:28   ` Zhu Yanjun
2023-10-13 13:01     ` Daisuke Matsuda (Fujitsu)
2023-10-13 13:44       ` Rain River
2023-10-16  6:07         ` Daisuke Matsuda (Fujitsu)
2023-10-18  8:34           ` Zhu Yanjun
2023-10-20  3:47   ` Zhijian Li (Fujitsu)
2023-10-20  6:54     ` Zhijian Li (Fujitsu)
2023-10-20 16:21       ` Bart Van Assche
2023-10-23  0:58         ` Zhijian Li (Fujitsu)
2023-10-20 14:01     ` Jason Gunthorpe
2023-10-23  3:52       ` Zhijian Li (Fujitsu)
2023-10-23  6:08         ` Zhu Yanjun
2023-10-23 10:45         ` Yi Zhang
2023-10-24  8:15           ` Zhijian Li (Fujitsu)
2023-10-24  9:13             ` Zhijian Li (Fujitsu)
2023-10-26  9:05               ` Zhijian Li (Fujitsu)
2023-10-26 11:42                 ` Jason Gunthorpe
2023-10-26 12:59                   ` Zhu Yanjun
2023-10-26 23:23                     ` Jason Gunthorpe
2023-10-27  1:36                       ` Zhu Yanjun
2023-10-27  4:01                       ` Zhu Yanjun
2023-10-27 11:51                         ` Jason Gunthorpe
2023-10-26 13:28                 ` Bart Van Assche
2023-10-26 13:43                   ` Jason Gunthorpe [this message]
2023-10-26 21:47                     ` Bart Van Assche
2023-10-27  1:26                       ` Daisuke Matsuda (Fujitsu)
2023-10-27  1:39                       ` Zhu Yanjun
2023-10-27  5:43                       ` Zhijian Li (Fujitsu)
2023-10-31  1:36                 ` Zhu Yanjun
     [not found]                   ` <CAEz=LcuLCe7bhUohh6BcHdJ1_ocJdZq=eu07vWb3Md5_ZOGDBg@mail.gmail.com>
     [not found]                     ` <CAEz=LcuQ6fFpHqBPT1oTUgKABAHFJqYDC-AHidE-+n6OtzmCPQ@mail.gmail.com>
2023-10-31  8:14                       ` Greg Sword

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231026134300.GV691768@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=bvanassche@acm.org \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=lizhijian@fujitsu.com \
    --cc=matsuda-daisuke@fujitsu.com \
    --cc=rpearsonhpe@gmail.com \
    --cc=yanjun.zhu@intel.com \
    --cc=yanjun.zhu@linux.dev \
    --cc=yi.zhang@redhat.com \
    --cc=zyjzyj2000@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox