| Message ID | 20231103095549.490744-1-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | rxe_map_mr_sg() fix cleanup and refactor | expand |
On Fri, Nov 3, 2023 at 5:58 PM Li Zhijian <lizhijian@fujitsu.com> wrote: > > I don't collect the Reviewed-by to the patch1-2 this time, since i > think we can make it better. > > Patch1-2: Fix kernel panic[1] and benifit to make srp work again. > Almost nothing change from V1. > Patch3-5: cleanups # newly add > Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested Do some work. Do not use these rubbish patch to waste our time. > > My bad arm64 mechine offten hangs when doing blktests even though i use the > default siw driver. > > - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. > > [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ > > Li Zhijian (6): > RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr > RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE > RDMA/rxe: remove unused rxe_mr.page_shift > RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from > page_list > RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} > RDMA/rxe: Support PAGE_SIZE aligned MR > > drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- > drivers/infiniband/sw/rxe/rxe_param.h | 2 +- > drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- > 3 files changed, 48 insertions(+), 43 deletions(-) > > -- > 2.41.0 >
在 2023/11/3 17:55, Li Zhijian 写道: > I don't collect the Reviewed-by to the patch1-2 this time, since i > think we can make it better. > > Patch1-2: Fix kernel panic[1] and benifit to make srp work again. > Almost nothing change from V1. > Patch3-5: cleanups # newly add > Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested > > My bad arm64 mechine offten hangs when doing blktests even though i use the > default siw driver. > > - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. Zhijian Please read carefully the whole discussion about this problem. You will find a lot of valuable suggestions, especially suggestions from Jason. From the whole discussion, it seems that the root cause is very clear. We need to fix this prolem. Please do not send this kind of commits again. Zhu Yanjun > > [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ > > Li Zhijian (6): > RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr > RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE > RDMA/rxe: remove unused rxe_mr.page_shift > RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from > page_list > RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} > RDMA/rxe: Support PAGE_SIZE aligned MR > > drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- > drivers/infiniband/sw/rxe/rxe_param.h | 2 +- > drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- > 3 files changed, 48 insertions(+), 43 deletions(-) >
On 03/11/2023 18:17, Greg Sword wrote: > On Fri, Nov 3, 2023 at 5:58 PM Li Zhijian <lizhijian@fujitsu.com> wrote: >> >> I don't collect the Reviewed-by to the patch1-2 this time, since i >> think we can make it better. >> >> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. >> Almost nothing change from V1. >> Patch3-5: cleanups # newly add >> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested > > Do some work. Do not use these rubbish patch to waste our time. So sorry about this. Of course, any other proposals are welcomed. > >> >> My bad arm64 mechine offten hangs when doing blktests even though i use the >> default siw driver. >> >> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. >> >> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ >> >> Li Zhijian (6): >> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr >> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE >> RDMA/rxe: remove unused rxe_mr.page_shift >> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from >> page_list >> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} >> RDMA/rxe: Support PAGE_SIZE aligned MR >> >> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- >> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- >> 3 files changed, 48 insertions(+), 43 deletions(-) >> >> -- >> 2.41.0 >>
On 03/11/2023 21:00, Zhu Yanjun wrote: > 在 2023/11/3 17:55, Li Zhijian 写道: >> I don't collect the Reviewed-by to the patch1-2 this time, since i >> think we can make it better. >> >> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. >> Almost nothing change from V1. >> Patch3-5: cleanups # newly add >> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested >> >> My bad arm64 mechine offten hangs when doing blktests even though i use the >> default siw driver. >> >> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. > > Zhijian > > Please read carefully the whole discussion about this problem. You will find a lot of valuable suggestions, especially suggestions from Jason. Okay, i will read it again. If you can tell me which thread, that would be better. > > From the whole discussion, it seems that the root cause is very clear. > We need to fix this prolem. Please do not send this kind of commits again. > Let's think about what's our goal first. - 1) Fix the panic[1] and only support PAGE_SIZE MR - 2) support PAGE_SIZE aligned MR - 3) support any page_size MR. I'm sorry i'm not familiar with the linux MM subsystem. It seem it's safe/correct to access address/memory across pages start from the return of kmap_loca_page(page). In other words, 2) is already native supported, right? I get totally confused now. > Zhu Yanjun > >> >> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ >> >> Li Zhijian (6): >> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr >> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE >> RDMA/rxe: remove unused rxe_mr.page_shift >> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from >> page_list >> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} >> RDMA/rxe: Support PAGE_SIZE aligned MR >> >> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- >> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- >> 3 files changed, 48 insertions(+), 43 deletions(-) >> >
Very thanks for all your feedback. On 03/11/2023 17:55, Li Zhijian wrote: > I don't collect the Reviewed-by to the patch1-2 this time, since i > think we can make it better. > > Patch1-2: Fix kernel panic[1] and benifit to make srp work again. > Almost nothing change from V1. Quote from Jason: " > The concept was that the xarray could store anything larger than > PAGE_SIZE and the entry would point at the first struct page of the > contiguous chunk > > That looks like it is right, or at least close to right, so lets try > to keep it " It seems it's okay to access address/memory across pages on RXE even though we only map the first page. That also means PAGE_SIZE aligned MR is already supported, so only check `if (IS_ALIGNED(page_size, PAGE_SIZE))` is sufficient, right? diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c index f54042e9aeb2..3755e530e6dc 100644 --- a/drivers/infiniband/sw/rxe/rxe_mr.c +++ b/drivers/infiniband/sw/rxe/rxe_mr.c @@ -234,6 +234,12 @@ int rxe_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sgl, struct rxe_mr *mr = to_rmr(ibmr); unsigned int page_size = mr_page_size(mr); + if (!IS_ALIGNED(page_size, PAGE_SIZE)) { + rxe_err_mr(mr, "FIXME...\n") + return -EINVAL; + } + mr->nbuf = 0; mr->page_shift = ilog2(page_size); mr->page_mask = ~((u64)page_size - 1); diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h index d2f57ead78ad..b1cf1e1c0ce1 100644 --- a/drivers/infiniband/sw/rxe/rxe_param.h +++ b/drivers/infiniband/sw/rxe/rxe_param.h @@ -38,7 +38,7 @@ static inline enum ib_mtu eth_mtu_int_to_enum(int mtu) /* default/initial rxe device parameter settings */ enum rxe_device_param { RXE_MAX_MR_SIZE = -1ull, - RXE_PAGE_SIZE_CAP = 0xfffff000, + RXE_PAGE_SIZE_CAP = 0xffffffff - (PAGE_SIZE - 1), RXE_MAX_QP_WR = DEFAULT_MAX_VALUE, RXE_DEVICE_CAP_FLAGS = IB_DEVICE_BAD_PKEY_CNTR | IB_DEVICE_BAD_QKEY_CNTR * minor cleanup will be done after this. Thanks Zhijian > Patch3-5: cleanups # newly add > Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested > > My bad arm64 mechine offten hangs when doing blktests even though i use the > default siw driver. > > - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. > > [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ > > Li Zhijian (6): > RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr > RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE > RDMA/rxe: remove unused rxe_mr.page_shift > RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from > page_list > RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} > RDMA/rxe: Support PAGE_SIZE aligned MR > > drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- > drivers/infiniband/sw/rxe/rxe_param.h | 2 +- > drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- > 3 files changed, 48 insertions(+), 43 deletions(-) >
On Mon, Nov 6, 2023 at 4:01 PM Zhijian Li (Fujitsu) <lizhijian@fujitsu.com> wrote: > > > > Very thanks for all your feedback. > > On 03/11/2023 17:55, Li Zhijian wrote: > > I don't collect the Reviewed-by to the patch1-2 this time, since i > > think we can make it better. > > > > Patch1-2: Fix kernel panic[1] and benifit to make srp work again. > > Almost nothing change from V1. > > Quote from Jason: > " > > The concept was that the xarray could store anything larger than > > PAGE_SIZE and the entry would point at the first struct page of the > > contiguous chunk > > > > That looks like it is right, or at least close to right, so lets try > > to keep it > " > > > It seems it's okay to access address/memory across pages on RXE even though > we only map the first page. Do you really make tests in your test environment? Do you have test environment? Do you really reproduce this problem in your test environment? Your patches do not work actually. Please do not send these rubbish patches out. > > That also means PAGE_SIZE aligned MR is already supported, so only check > `if (IS_ALIGNED(page_size, PAGE_SIZE))` is sufficient, right? > > diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c > index f54042e9aeb2..3755e530e6dc 100644 > --- a/drivers/infiniband/sw/rxe/rxe_mr.c > +++ b/drivers/infiniband/sw/rxe/rxe_mr.c > @@ -234,6 +234,12 @@ int rxe_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sgl, > struct rxe_mr *mr = to_rmr(ibmr); > unsigned int page_size = mr_page_size(mr); > > + if (!IS_ALIGNED(page_size, PAGE_SIZE)) { > + rxe_err_mr(mr, "FIXME...\n") > + return -EINVAL; > + } > + > mr->nbuf = 0; > mr->page_shift = ilog2(page_size); > mr->page_mask = ~((u64)page_size - 1); > diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h > index d2f57ead78ad..b1cf1e1c0ce1 100644 > --- a/drivers/infiniband/sw/rxe/rxe_param.h > +++ b/drivers/infiniband/sw/rxe/rxe_param.h > @@ -38,7 +38,7 @@ static inline enum ib_mtu eth_mtu_int_to_enum(int mtu) > /* default/initial rxe device parameter settings */ > enum rxe_device_param { > RXE_MAX_MR_SIZE = -1ull, > - RXE_PAGE_SIZE_CAP = 0xfffff000, > + RXE_PAGE_SIZE_CAP = 0xffffffff - (PAGE_SIZE - 1), > RXE_MAX_QP_WR = DEFAULT_MAX_VALUE, > RXE_DEVICE_CAP_FLAGS = IB_DEVICE_BAD_PKEY_CNTR > | IB_DEVICE_BAD_QKEY_CNTR > > > * minor cleanup will be done after this. > > Thanks > Zhijian > > > Patch3-5: cleanups # newly add > > Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested > > > > My bad arm64 mechine offten hangs when doing blktests even though i use the > > default siw driver. > > > > - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. > > > > [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ > > > > Li Zhijian (6): > > RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr > > RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE > > RDMA/rxe: remove unused rxe_mr.page_shift > > RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from > > page_list > > RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} > > RDMA/rxe: Support PAGE_SIZE aligned MR > > > > drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- > > drivers/infiniband/sw/rxe/rxe_param.h | 2 +- > > drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- > > 3 files changed, 48 insertions(+), 43 deletions(-) > >
On 06/11/2023 17:35, Greg Sword wrote: > On Mon, Nov 6, 2023 at 4:01 PM Zhijian Li (Fujitsu) > <lizhijian@fujitsu.com> wrote: >> >> >> >> Very thanks for all your feedback. >> >> On 03/11/2023 17:55, Li Zhijian wrote: >>> I don't collect the Reviewed-by to the patch1-2 this time, since i >>> think we can make it better. >>> >>> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. >>> Almost nothing change from V1. >> >> Quote from Jason: >> " >>> The concept was that the xarray could store anything larger than >>> PAGE_SIZE and the entry would point at the first struct page of the >>> contiguous chunk >>> >>> That looks like it is right, or at least close to right, so lets try >>> to keep it >> " >> >> >> It seems it's okay to access address/memory across pages on RXE even though >> we only map the first page. > > Do you really make tests in your test environment? Do you have test environment? > Do you really reproduce this problem in your test environment? I did the test, the kernel panic[1] is gone after patch1-patch2 Thanks Zhijian > Your patches do not work actually. Please do not send these rubbish patches out. > >> >> That also means PAGE_SIZE aligned MR is already supported, so only check >> `if (IS_ALIGNED(page_size, PAGE_SIZE))` is sufficient, right? >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c >> index f54042e9aeb2..3755e530e6dc 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_mr.c >> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c >> @@ -234,6 +234,12 @@ int rxe_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sgl, >> struct rxe_mr *mr = to_rmr(ibmr); >> unsigned int page_size = mr_page_size(mr); >> >> + if (!IS_ALIGNED(page_size, PAGE_SIZE)) { >> + rxe_err_mr(mr, "FIXME...\n") >> + return -EINVAL; >> + } >> + >> mr->nbuf = 0; >> mr->page_shift = ilog2(page_size); >> mr->page_mask = ~((u64)page_size - 1); >> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h >> index d2f57ead78ad..b1cf1e1c0ce1 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_param.h >> +++ b/drivers/infiniband/sw/rxe/rxe_param.h >> @@ -38,7 +38,7 @@ static inline enum ib_mtu eth_mtu_int_to_enum(int mtu) >> /* default/initial rxe device parameter settings */ >> enum rxe_device_param { >> RXE_MAX_MR_SIZE = -1ull, >> - RXE_PAGE_SIZE_CAP = 0xfffff000, >> + RXE_PAGE_SIZE_CAP = 0xffffffff - (PAGE_SIZE - 1), >> RXE_MAX_QP_WR = DEFAULT_MAX_VALUE, >> RXE_DEVICE_CAP_FLAGS = IB_DEVICE_BAD_PKEY_CNTR >> | IB_DEVICE_BAD_QKEY_CNTR >> >> >> * minor cleanup will be done after this. >> >> Thanks >> Zhijian >> >>> Patch3-5: cleanups # newly add >>> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested >>> >>> My bad arm64 mechine offten hangs when doing blktests even though i use the >>> default siw driver. >>> >>> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. >>> >>> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ >>> >>> Li Zhijian (6): >>> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr >>> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE >>> RDMA/rxe: remove unused rxe_mr.page_shift >>> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from >>> page_list >>> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} >>> RDMA/rxe: Support PAGE_SIZE aligned MR >>> >>> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- >>> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >>> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- >>> 3 files changed, 48 insertions(+), 43 deletions(-) >>>
在 2023/11/6 12:07, Zhijian Li (Fujitsu) 写道: > > > On 03/11/2023 21:00, Zhu Yanjun wrote: >> 在 2023/11/3 17:55, Li Zhijian 写道: >>> I don't collect the Reviewed-by to the patch1-2 this time, since i >>> think we can make it better. >>> >>> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. >>> Almost nothing change from V1. >>> Patch3-5: cleanups # newly add >>> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested >>> >>> My bad arm64 mechine offten hangs when doing blktests even though i use the >>> default siw driver. >>> >>> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. >> >> Zhijian >> >> Please read carefully the whole discussion about this problem. You will find a lot of valuable suggestions, especially suggestions from Jason. > > Okay, i will read it again. If you can tell me which thread, that would be better. > > >> >> From the whole discussion, it seems that the root cause is very clear. >> We need to fix this prolem. Please do not send this kind of commits again. >> > > Let's think about what's our goal first. > > - 1) Fix the panic[1] and only support PAGE_SIZE MR > - 2) support PAGE_SIZE aligned MR > - 3) support any page_size MR. > > I'm sorry i'm not familiar with the linux MM subsystem. It seem it's safe/correct to access > address/memory across pages start from the return of kmap_loca_page(page). > In other words, 2) is already native supported, right? Yes. Please read the comments from Jason, Leon and Bart. They shared a lot of good advice. From them, we can know the root cause and how to fix this problem. Good Luck. Zhu Yanjun > > I get totally confused now. > > > >> Zhu Yanjun >> >>> >>> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ >>> >>> Li Zhijian (6): >>> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr >>> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE >>> RDMA/rxe: remove unused rxe_mr.page_shift >>> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from >>> page_list >>> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} >>> RDMA/rxe: Support PAGE_SIZE aligned MR >>> >>> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- >>> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >>> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- >>> 3 files changed, 48 insertions(+), 43 deletions(-) >>>
On Mon, Nov 06, 2023 at 04:07:19AM +0000, Zhijian Li (Fujitsu) wrote: > I'm sorry i'm not familiar with the linux MM subsystem. It seem it's safe/correct to access > address/memory across pages start from the return of > kmap_loca_page(page). kmap_local_page() gives you a PAGE_SIZE window only Jason
On 06/11/2023 21:58, Zhu Yanjun wrote: > 在 2023/11/6 12:07, Zhijian Li (Fujitsu) 写道: >> >> >> On 03/11/2023 21:00, Zhu Yanjun wrote: >>> 在 2023/11/3 17:55, Li Zhijian 写道: >>>> I don't collect the Reviewed-by to the patch1-2 this time, since i >>>> think we can make it better. >>>> >>>> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. >>>> Almost nothing change from V1. >>>> Patch3-5: cleanups # newly add >>>> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested >>>> >>>> My bad arm64 mechine offten hangs when doing blktests even though i use the >>>> default siw driver. >>>> >>>> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. >>> >>> Zhijian >>> >>> Please read carefully the whole discussion about this problem. You will find a lot of valuable suggestions, especially suggestions from Jason. >> >> Okay, i will read it again. If you can tell me which thread, that would be better. >> >> >>> >>> From the whole discussion, it seems that the root cause is very clear. >>> We need to fix this prolem. Please do not send this kind of commits again. >>> >> >> Let's think about what's our goal first. >> >> - 1) Fix the panic[1] and only support PAGE_SIZE MR >> - 2) support PAGE_SIZE aligned MR >> - 3) support any page_size MR. >> >> I'm sorry i'm not familiar with the linux MM subsystem. It seem it's safe/correct to access >> address/memory across pages start from the return of kmap_loca_page(page). >> In other words, 2) is already native supported, right? > > Yes. Please read the comments from Jason, Leon and Bart. They shared a lot of good advice. I read the whole discussion again, but I believed i still missed a lost. > From them, we can know the root cause and how to fix this problem. I don't think i misunderstood the root cause: RXE splits memory into PAGE_SIZE units in the xarray. As a result, when we extract an address from the xarray, we should not access address beyond a PAGE_SIZE window. IIUC, then how to fix it? - I'm not going to "removing page_size set", it's out of this patch scope. Feel free to do the cleanup separately. - I'm not going to fix the NVMe/rtrs etc problems in this patch set when 64K page is enabled. But RXE will tell its callers explicitly "RXE don't don't support such page_size" - I didn't state RXE supports PAGE_SIZE aligned page_size MR before refactoring rxe_map_mr_sg(), because I worry about it was not correct to access address beyond the PAGE_SIZE window. What I should do next? Just state "RXE support PAGE_SIZE aligned MR" ? Then patches become RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE aligned MR RDMA/rxe: set RXE_PAGE_SIZE_CAP to starting from PAGE_SIZE Or just keep we have done in the V1 Thanks > > Good Luck. > > Zhu Yanjun > >> >> I get totally confused now. >> >> >> >>> Zhu Yanjun >>> >>>> >>>> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ >>>> >>>> Li Zhijian (6): >>>> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr >>>> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE >>>> RDMA/rxe: remove unused rxe_mr.page_shift >>>> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from >>>> page_list >>>> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} >>>> RDMA/rxe: Support PAGE_SIZE aligned MR >>>> >>>> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- >>>> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >>>> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- >>>> 3 files changed, 48 insertions(+), 43 deletions(-) >>>> >
在 2023/11/9 10:24, Zhijian Li (Fujitsu) 写道: > > On 06/11/2023 21:58, Zhu Yanjun wrote: >> 在 2023/11/6 12:07, Zhijian Li (Fujitsu) 写道: >>> >>> On 03/11/2023 21:00, Zhu Yanjun wrote: >>>> 在 2023/11/3 17:55, Li Zhijian 写道: >>>>> I don't collect the Reviewed-by to the patch1-2 this time, since i >>>>> think we can make it better. >>>>> >>>>> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. >>>>> Almost nothing change from V1. >>>>> Patch3-5: cleanups # newly add >>>>> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested >>>>> >>>>> My bad arm64 mechine offten hangs when doing blktests even though i use the >>>>> default siw driver. >>>>> >>>>> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. >>>> Zhijian >>>> >>>> Please read carefully the whole discussion about this problem. You will find a lot of valuable suggestions, especially suggestions from Jason. >>> Okay, i will read it again. If you can tell me which thread, that would be better. >>> >>> >>>> From the whole discussion, it seems that the root cause is very clear. >>>> We need to fix this prolem. Please do not send this kind of commits again. >>>> >>> Let's think about what's our goal first. >>> >>> - 1) Fix the panic[1] and only support PAGE_SIZE MR >>> - 2) support PAGE_SIZE aligned MR >>> - 3) support any page_size MR. >>> >>> I'm sorry i'm not familiar with the linux MM subsystem. It seem it's safe/correct to access >>> address/memory across pages start from the return of kmap_loca_page(page). >>> In other words, 2) is already native supported, right? >> Yes. Please read the comments from Jason, Leon and Bart. They shared a lot of good advice. > I read the whole discussion again, but I believed i still missed a lost. > > >> From them, we can know the root cause and how to fix this problem. > I don't think i misunderstood the root cause: > RXE splits memory into PAGE_SIZE units in the xarray. As a result, when we extract an address from the xarray, > we should not access address beyond a PAGE_SIZE window. This is a complicated problem and it is deeply involved with memory management. A guy who is very familiar with linux MM is to provide a better solution to this problem. I expect a whole perfect solution to this problem. Zhu Yanjun > > IIUC, then how to fix it? > - I'm not going to "removing page_size set", it's out of this patch scope. > Feel free to do the cleanup separately. > - I'm not going to fix the NVMe/rtrs etc problems in this patch set when 64K page is enabled. > But RXE will tell its callers explicitly "RXE don't don't support such page_size" > - I didn't state RXE supports PAGE_SIZE aligned page_size MR before refactoring rxe_map_mr_sg(), > because I worry about it was not correct to access address beyond the PAGE_SIZE window. > > What I should do next? > Just state "RXE support PAGE_SIZE aligned MR" ? Then patches become > RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE aligned MR > RDMA/rxe: set RXE_PAGE_SIZE_CAP to starting from PAGE_SIZE > > Or just keep we have done in the V1 > > Thanks > > >> Good Luck. >> >> Zhu Yanjun >> >>> I get totally confused now. >>> >>> >>> >>>> Zhu Yanjun >>>> >>>>> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ >>>>> >>>>> Li Zhijian (6): >>>>> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr >>>>> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE >>>>> RDMA/rxe: remove unused rxe_mr.page_shift >>>>> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from >>>>> page_list >>>>> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} >>>>> RDMA/rxe: Support PAGE_SIZE aligned MR >>>>> >>>>> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- >>>>> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >>>>> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- >>>>> 3 files changed, 48 insertions(+), 43 deletions(-) >>>>>
On Thu, Nov 9, 2023 at 10:25 AM Zhijian Li (Fujitsu) <lizhijian@fujitsu.com> wrote: > > > > On 06/11/2023 21:58, Zhu Yanjun wrote: > > 在 2023/11/6 12:07, Zhijian Li (Fujitsu) 写道: > >> > >> > >> On 03/11/2023 21:00, Zhu Yanjun wrote: > >>> 在 2023/11/3 17:55, Li Zhijian 写道: > >>>> I don't collect the Reviewed-by to the patch1-2 this time, since i > >>>> think we can make it better. > >>>> > >>>> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. > >>>> Almost nothing change from V1. > >>>> Patch3-5: cleanups # newly add > >>>> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested > >>>> > >>>> My bad arm64 mechine offten hangs when doing blktests even though i use the > >>>> default siw driver. > >>>> > >>>> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. > >>> > >>> Zhijian > >>> > >>> Please read carefully the whole discussion about this problem. You will find a lot of valuable suggestions, especially suggestions from Jason. > >> > >> Okay, i will read it again. If you can tell me which thread, that would be better. > >> > >> > >>> > >>> From the whole discussion, it seems that the root cause is very clear. > >>> We need to fix this prolem. Please do not send this kind of commits again. > >>> > >> > >> Let's think about what's our goal first. > >> > >> - 1) Fix the panic[1] and only support PAGE_SIZE MR > >> - 2) support PAGE_SIZE aligned MR > >> - 3) support any page_size MR. > >> > >> I'm sorry i'm not familiar with the linux MM subsystem. It seem it's safe/correct to access > >> address/memory across pages start from the return of kmap_loca_page(page). > >> In other words, 2) is already native supported, right? > > > > Yes. Please read the comments from Jason, Leon and Bart. They shared a lot of good advice. > > I read the whole discussion again, but I believed i still missed a lost. > > > > From them, we can know the root cause and how to fix this problem. > > I don't think i misunderstood the root cause: > RXE splits memory into PAGE_SIZE units in the xarray. As a result, when we extract an address from the xarray, > we should not access address beyond a PAGE_SIZE window. > > IIUC, then how to fix it? > - I'm not going to "removing page_size set", it's out of this patch scope. > Feel free to do the cleanup separately. > - I'm not going to fix the NVMe/rtrs etc problems in this patch set when 64K page is enabled. > But RXE will tell its callers explicitly "RXE don't don't support such page_size" > - I didn't state RXE supports PAGE_SIZE aligned page_size MR before refactoring rxe_map_mr_sg(), > because I worry about it was not correct to access address beyond the PAGE_SIZE window. > > What I should do next? > Just state "RXE support PAGE_SIZE aligned MR" ? Then patches become > RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE aligned MR > RDMA/rxe: set RXE_PAGE_SIZE_CAP to starting from PAGE_SIZE > What do you take rdma maillist for? Your bugzilla, jira? or your dev program launch? Or your play ground? > Or just keep we have done in the V1 > > Thanks > > > > > > Good Luck. > > > > Zhu Yanjun > > > >> > >> I get totally confused now. > >> > >> > >> > >>> Zhu Yanjun > >>> > >>>> > >>>> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ > >>>> > >>>> Li Zhijian (6): > >>>> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr > >>>> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE > >>>> RDMA/rxe: remove unused rxe_mr.page_shift > >>>> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from > >>>> page_list > >>>> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} > >>>> RDMA/rxe: Support PAGE_SIZE aligned MR > >>>> > >>>> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- > >>>> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- > >>>> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- > >>>> 3 files changed, 48 insertions(+), 43 deletions(-) > >>>> > >
On 09/11/2023 15:16, Greg Sword wrote: > On Thu, Nov 9, 2023 at 10:25 AM Zhijian Li (Fujitsu) > <lizhijian@fujitsu.com> wrote: >> >> >> >> On 06/11/2023 21:58, Zhu Yanjun wrote: >>> 在 2023/11/6 12:07, Zhijian Li (Fujitsu) 写道: >>>> >>>> >>>> On 03/11/2023 21:00, Zhu Yanjun wrote: >>>>> 在 2023/11/3 17:55, Li Zhijian 写道: >>>>>> I don't collect the Reviewed-by to the patch1-2 this time, since i >>>>>> think we can make it better. >>>>>> >>>>>> Patch1-2: Fix kernel panic[1] and benifit to make srp work again. >>>>>> Almost nothing change from V1. >>>>>> Patch3-5: cleanups # newly add >>>>>> Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested >>>>>> >>>>>> My bad arm64 mechine offten hangs when doing blktests even though i use the >>>>>> default siw driver. >>>>>> >>>>>> - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. >>>>> >>>>> Zhijian >>>>> >>>>> Please read carefully the whole discussion about this problem. You will find a lot of valuable suggestions, especially suggestions from Jason. >>>> >>>> Okay, i will read it again. If you can tell me which thread, that would be better. >>>> >>>> >>>>> >>>>> From the whole discussion, it seems that the root cause is very clear. >>>>> We need to fix this prolem. Please do not send this kind of commits again. >>>>> >>>> >>>> Let's think about what's our goal first. >>>> >>>> - 1) Fix the panic[1] and only support PAGE_SIZE MR >>>> - 2) support PAGE_SIZE aligned MR >>>> - 3) support any page_size MR. >>>> >>>> I'm sorry i'm not familiar with the linux MM subsystem. It seem it's safe/correct to access >>>> address/memory across pages start from the return of kmap_loca_page(page). >>>> In other words, 2) is already native supported, right? >>> >>> Yes. Please read the comments from Jason, Leon and Bart. They shared a lot of good advice. >> >> I read the whole discussion again, but I believed i still missed a lost. >> >> >>> From them, we can know the root cause and how to fix this problem. >> >> I don't think i misunderstood the root cause: >> RXE splits memory into PAGE_SIZE units in the xarray. As a result, when we extract an address from the xarray, >> we should not access address beyond a PAGE_SIZE window. >> >> IIUC, then how to fix it? >> - I'm not going to "removing page_size set", it's out of this patch scope. >> Feel free to do the cleanup separately. >> - I'm not going to fix the NVMe/rtrs etc problems in this patch set when 64K page is enabled. >> But RXE will tell its callers explicitly "RXE don't don't support such page_size" >> - I didn't state RXE supports PAGE_SIZE aligned page_size MR before refactoring rxe_map_mr_sg(), >> because I worry about it was not correct to access address beyond the PAGE_SIZE window. >> >> What I should do next? >> Just state "RXE support PAGE_SIZE aligned MR" ? Then patches become >> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE aligned MR >> RDMA/rxe: set RXE_PAGE_SIZE_CAP to starting from PAGE_SIZE >> > > What do you take rdma maillist for? Your bugzilla, jira? or your dev > program launch? Or your play ground? May i know which bug you are concerning. Actually i always cannot get your point. > >> Or just keep we have done in the V1 >> >> Thanks >> >> >>> >>> Good Luck. >>> >>> Zhu Yanjun >>> >>>> >>>> I get totally confused now. >>>> >>>> >>>> >>>>> Zhu Yanjun >>>>> >>>>>> >>>>>> [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ >>>>>> >>>>>> Li Zhijian (6): >>>>>> RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr >>>>>> RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE >>>>>> RDMA/rxe: remove unused rxe_mr.page_shift >>>>>> RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from >>>>>> page_list >>>>>> RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} >>>>>> RDMA/rxe: Support PAGE_SIZE aligned MR >>>>>> >>>>>> drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- >>>>>> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >>>>>> drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- >>>>>> 3 files changed, 48 insertions(+), 43 deletions(-) >>>>>> >>>
On Thu, Nov 09, 2023 at 03:16:58PM +0800, Greg Sword wrote: > > What I should do next? > > Just state "RXE support PAGE_SIZE aligned MR" ? Then patches become > > RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE aligned MR > > RDMA/rxe: set RXE_PAGE_SIZE_CAP to starting from PAGE_SIZE > > What do you take rdma maillist for? Your bugzilla, jira? or your dev > program launch? Or your play ground? We have a code of conduct on these mailing lists, please follow it or stop posting. Jason
I don't collect the Reviewed-by to the patch1-2 this time, since i think we can make it better. Patch1-2: Fix kernel panic[1] and benifit to make srp work again. Almost nothing change from V1. Patch3-5: cleanups # newly add Patch6: make RXE support PAGE_SIZE aligned mr # newly add, but not fully tested My bad arm64 mechine offten hangs when doing blktests even though i use the default siw driver. - nvme and ULPs(rtrs, iser) always registers 4K mr still don't supported yet. [1] https://lore.kernel.org/all/CAHj4cs9XRqE25jyVw9rj9YugffLn5+f=1znaBEnu1usLOciD+g@mail.gmail.com/T/ Li Zhijian (6): RDMA/rxe: RDMA/rxe: don't allow registering !PAGE_SIZE mr RDMA/rxe: set RXE_PAGE_SIZE_CAP to PAGE_SIZE RDMA/rxe: remove unused rxe_mr.page_shift RDMA/rxe: Use PAGE_SIZE and PAGE_SHIFT to extract address from page_list RDMA/rxe: cleanup rxe_mr.{page_size,page_shift} RDMA/rxe: Support PAGE_SIZE aligned MR drivers/infiniband/sw/rxe/rxe_mr.c | 80 ++++++++++++++++----------- drivers/infiniband/sw/rxe/rxe_param.h | 2 +- drivers/infiniband/sw/rxe/rxe_verbs.h | 9 --- 3 files changed, 48 insertions(+), 43 deletions(-)