mbox series

[for-next,0/7] FIXME and other fixes

Message ID 167328561962.1472990.9463955313515395755.stgit@awfm-02.cornelisnetworks.com (mailing list archive)
Headers show
Series FIXME and other fixes | expand

Message

Dennis Dalessandro Jan. 9, 2023, 7:03 p.m. UTC
Dean fixes the FIXME that was left by Jason in the code to use the interval
notifier. There are also fixes for other things like splitting our counter
allocation to better align with the way the core is moving. 

---

Dean Luick (7):
      IB/hfi1: Remove redundant pageidx variable
      IB/hfi1: Assign npages earlier
      IB/hfi1: Consolidate the creation of user TIDs
      IB/hfi1: Improve TID validity checking
      IB/hfi1: Split IB counter allocation
      IBh/hfi1: Update RMT size calculation
      IB/hfi1: Use dma_mmap_coherent for matching buffers


 drivers/infiniband/hw/hfi1/chip.c         | 59 +++++++++--------
 drivers/infiniband/hw/hfi1/exp_rcv.h      |  5 +-
 drivers/infiniband/hw/hfi1/file_ops.c     | 81 ++++++++++++++++-------
 drivers/infiniband/hw/hfi1/user_exp_rcv.c | 55 ++++++---------
 drivers/infiniband/hw/hfi1/verbs.c        | 81 +++++++++--------------
 5 files changed, 146 insertions(+), 135 deletions(-)

--
-Denny

Comments

Leon Romanovsky Jan. 10, 2023, 10:53 a.m. UTC | #1
On Mon, 09 Jan 2023 14:03:58 -0500, Dennis Dalessandro wrote:
> Dean fixes the FIXME that was left by Jason in the code to use the interval
> notifier. There are also fixes for other things like splitting our counter
> allocation to better align with the way the core is moving.
> 

Applied, thanks!

[1/7] IB/hfi1: Remove redundant pageidx variable
      https://git.kernel.org/rdma/rdma/c/3c49eef3897822
[2/7] IB/hfi1: Assign npages earlier
      https://git.kernel.org/rdma/rdma/c/a479433a6b7a2b
[3/7] IB/hfi1: Consolidate the creation of user TIDs
      https://git.kernel.org/rdma/rdma/c/d8f4ab01c6d0d5
[4/7] IB/hfi1: Improve TID validity checking
      https://git.kernel.org/rdma/rdma/c/845127ed8717e0
[5/7] IB/hfi1: Split IB counter allocation
      https://git.kernel.org/rdma/rdma/c/ef90f0a1913e8b
[6/7] IBh/hfi1: Update RMT size calculation
      https://git.kernel.org/rdma/rdma/c/131268e2558f1f
[7/7] IB/hfi1: Use dma_mmap_coherent for matching buffers
      https://git.kernel.org/rdma/rdma/c/5d3fcb45a374a7

Best regards,
Jason Gunthorpe Jan. 10, 2023, 2:58 p.m. UTC | #2
On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
> Dean fixes the FIXME that was left by Jason in the code to use the interval
> notifier.

? Which patch did that?

Jason
Dennis Dalessandro Jan. 10, 2023, 9:03 p.m. UTC | #3
On 1/10/23 9:58 AM, Jason Gunthorpe wrote:
> On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
>> Dean fixes the FIXME that was left by Jason in the code to use the interval
>> notifier.
> 
> ? Which patch did that?

My fault. The last patch in the previous series [1] was meant to go first here.
I got off by 1 when I was splitting the patches out for submit.

[1]
https://lore.kernel.org/linux-rdma/167328549178.1472310.9867497376936699488.stgit@awfm-02.cornelisnetworks.com/T/#u

-Denny
Dennis Dalessandro Jan. 13, 2023, 5:21 p.m. UTC | #4
On 1/10/23 4:03 PM, Dennis Dalessandro wrote:
> On 1/10/23 9:58 AM, Jason Gunthorpe wrote:
>> On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
>>> Dean fixes the FIXME that was left by Jason in the code to use the interval
>>> notifier.
>>
>> ? Which patch did that?
> 
> My fault. The last patch in the previous series [1] was meant to go first here.
> I got off by 1 when I was splitting the patches out for submit.
> 
> [1]
> https://lore.kernel.org/linux-rdma/167328549178.1472310.9867497376936699488.stgit@awfm-02.cornelisnetworks.com/T/#u

As a side effect of this, can we pull patch 2/7 from this series into the RC?

[PATCH for-next 2/7] IB/hfi1: Assign npages earlier

should go with:

[PATCH for-rc 6/6] IB/hfi1: Remove user expected buffer invalidate race

-Denny
Leon Romanovsky Jan. 15, 2023, 11:46 a.m. UTC | #5
On Fri, Jan 13, 2023 at 12:21:50PM -0500, Dennis Dalessandro wrote:
> On 1/10/23 4:03 PM, Dennis Dalessandro wrote:
> > On 1/10/23 9:58 AM, Jason Gunthorpe wrote:
> >> On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
> >>> Dean fixes the FIXME that was left by Jason in the code to use the interval
> >>> notifier.
> >>
> >> ? Which patch did that?
> > 
> > My fault. The last patch in the previous series [1] was meant to go first here.
> > I got off by 1 when I was splitting the patches out for submit.
> > 
> > [1]
> > https://lore.kernel.org/linux-rdma/167328549178.1472310.9867497376936699488.stgit@awfm-02.cornelisnetworks.com/T/#u
> 
> As a side effect of this, can we pull patch 2/7 from this series into the RC?

No, everything is in for-rc/for-next now.

Thanks
Dennis Dalessandro Jan. 16, 2023, 5:36 a.m. UTC | #6
On 1/15/23 6:46 AM, Leon Romanovsky wrote:
> On Fri, Jan 13, 2023 at 12:21:50PM -0500, Dennis Dalessandro wrote:
>> On 1/10/23 4:03 PM, Dennis Dalessandro wrote:
>>> On 1/10/23 9:58 AM, Jason Gunthorpe wrote:
>>>> On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
>>>>> Dean fixes the FIXME that was left by Jason in the code to use the interval
>>>>> notifier.
>>>>
>>>> ? Which patch did that?
>>>
>>> My fault. The last patch in the previous series [1] was meant to go first here.
>>> I got off by 1 when I was splitting the patches out for submit.
>>>
>>> [1]
>>> https://lore.kernel.org/linux-rdma/167328549178.1472310.9867497376936699488.stgit@awfm-02.cornelisnetworks.com/T/#u
>>
>> As a side effect of this, can we pull patch 2/7 from this series into the RC?
> 
> No, everything is in for-rc/for-next now.

Without that patch there will be a regression in 6.2. Is there a reason it can't
merge into -rc?

-Denny
Leon Romanovsky Jan. 16, 2023, 7:16 a.m. UTC | #7
On Mon, Jan 16, 2023 at 12:36:51AM -0500, Dennis Dalessandro wrote:
> On 1/15/23 6:46 AM, Leon Romanovsky wrote:
> > On Fri, Jan 13, 2023 at 12:21:50PM -0500, Dennis Dalessandro wrote:
> >> On 1/10/23 4:03 PM, Dennis Dalessandro wrote:
> >>> On 1/10/23 9:58 AM, Jason Gunthorpe wrote:
> >>>> On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
> >>>>> Dean fixes the FIXME that was left by Jason in the code to use the interval
> >>>>> notifier.
> >>>>
> >>>> ? Which patch did that?
> >>>
> >>> My fault. The last patch in the previous series [1] was meant to go first here.
> >>> I got off by 1 when I was splitting the patches out for submit.
> >>>
> >>> [1]
> >>> https://lore.kernel.org/linux-rdma/167328549178.1472310.9867497376936699488.stgit@awfm-02.cornelisnetworks.com/T/#u
> >>
> >> As a side effect of this, can we pull patch 2/7 from this series into the RC?
> > 
> > No, everything is in for-rc/for-next now.
> 
> Without that patch there will be a regression in 6.2. 

I'm lost here. You are saying above that you wanted patch from -rc to be
in -next series. However, you wrote about regression in 6.2, which is -rc.

> Is there a reason it can't merge into -rc?

Here you are asking to bring -next patches to -rc.

So please help me, what do you want to do with these branches?
1. -rc
2. -next

Options are:
1. keep as is
2. revert
3. anything else?

What we won't do:
1. backmerge -next to -rc
2. merge -rc into -next without strong justification, as it is not
needed in general because such merge happens during merge window.

Thanks
Dennis Dalessandro Jan. 18, 2023, 1:04 p.m. UTC | #8
On 1/16/23 2:16 AM, Leon Romanovsky wrote:
> On Mon, Jan 16, 2023 at 12:36:51AM -0500, Dennis Dalessandro wrote:
>> On 1/15/23 6:46 AM, Leon Romanovsky wrote:
>>> On Fri, Jan 13, 2023 at 12:21:50PM -0500, Dennis Dalessandro wrote:
>>>> On 1/10/23 4:03 PM, Dennis Dalessandro wrote:
>>>>> On 1/10/23 9:58 AM, Jason Gunthorpe wrote:
>>>>>> On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
>>>>>>> Dean fixes the FIXME that was left by Jason in the code to use the interval
>>>>>>> notifier.
>>>>>>
>>>>>> ? Which patch did that?
>>>>>
>>>>> My fault. The last patch in the previous series [1] was meant to go first here.
>>>>> I got off by 1 when I was splitting the patches out for submit.
>>>>>
>>>>> [1]
>>>>> https://lore.kernel.org/linux-rdma/167328549178.1472310.9867497376936699488.stgit@awfm-02.cornelisnetworks.com/T/#u
>>>>
>>>> As a side effect of this, can we pull patch 2/7 from this series into the RC?
>>>
>>> No, everything is in for-rc/for-next now.
>>
>> Without that patch there will be a regression in 6.2. 
> 
> I'm lost here. You are saying above that you wanted patch from -rc to be
> in -next series. However, you wrote about regression in 6.2, which is -rc.

Originally I did not mean to send:
[PATCH for-rc 6/6] IB/hfi1: Remove user expected buffer invalidate race
for -rc.

I didn't realize, it has a functional dependency on:
[PATCH for-next 2/7] IB/hfi1: Assign npages earlier

Ideally either they both go to -rc or they both go to -next.

>> Is there a reason it can't merge into -rc?
> 
> Here you are asking to bring -next patches to -rc.

One patch.

> So please help me, what do you want to do with these branches?
> 1. -rc
> 2. -next
> 
> Options are:
> 1. keep as is
> 2. revert

Let me do some build testing. If we revert the -rc patch and then reapply to
-next we may encounter conflicts and/or build issues and just make things worse.

> 3. anything else?Will get back to you if I come up with something else.

> What we won't do:
> 1. backmerge -next to -rc

So why is this not an option? Well ok so I don't mean we should merge. I guess
I'm more looking to cherry-pick.

> 2. merge -rc into -next without strong justification, as it is not
> needed in general because such merge happens during merge window.

Agree, not needed.

-Denny
Leon Romanovsky Jan. 20, 2023, 5:09 p.m. UTC | #9
On Wed, Jan 18, 2023 at 08:04:52AM -0500, Dennis Dalessandro wrote:
> On 1/16/23 2:16 AM, Leon Romanovsky wrote:
> > On Mon, Jan 16, 2023 at 12:36:51AM -0500, Dennis Dalessandro wrote:
> >> On 1/15/23 6:46 AM, Leon Romanovsky wrote:
> >>> On Fri, Jan 13, 2023 at 12:21:50PM -0500, Dennis Dalessandro wrote:
> >>>> On 1/10/23 4:03 PM, Dennis Dalessandro wrote:
> >>>>> On 1/10/23 9:58 AM, Jason Gunthorpe wrote:
> >>>>>> On Mon, Jan 09, 2023 at 02:03:58PM -0500, Dennis Dalessandro wrote:
> >>>>>>> Dean fixes the FIXME that was left by Jason in the code to use the interval
> >>>>>>> notifier.
> >>>>>>
> >>>>>> ? Which patch did that?
> >>>>>
> >>>>> My fault. The last patch in the previous series [1] was meant to go first here.
> >>>>> I got off by 1 when I was splitting the patches out for submit.
> >>>>>
> >>>>> [1]
> >>>>> https://lore.kernel.org/linux-rdma/167328549178.1472310.9867497376936699488.stgit@awfm-02.cornelisnetworks.com/T/#u
> >>>>
> >>>> As a side effect of this, can we pull patch 2/7 from this series into the RC?
> >>>
> >>> No, everything is in for-rc/for-next now.
> >>
> >> Without that patch there will be a regression in 6.2. 
> > 
> > I'm lost here. You are saying above that you wanted patch from -rc to be
> > in -next series. However, you wrote about regression in 6.2, which is -rc.
> 
> Originally I did not mean to send:
> [PATCH for-rc 6/6] IB/hfi1: Remove user expected buffer invalidate race
> for -rc.
> 
> I didn't realize, it has a functional dependency on:
> [PATCH for-next 2/7] IB/hfi1: Assign npages earlier
> 
> Ideally either they both go to -rc or they both go to -next.
> 
> >> Is there a reason it can't merge into -rc?
> > 
> > Here you are asking to bring -next patches to -rc.
> 
> One patch.
> 
> > So please help me, what do you want to do with these branches?
> > 1. -rc
> > 2. -next
> > 
> > Options are:
> > 1. keep as is
> > 2. revert
> 
> Let me do some build testing. If we revert the -rc patch and then reapply to
> -next we may encounter conflicts and/or build issues and just make things worse.
> 
> > 3. anything else?Will get back to you if I come up with something else.
> 
> > What we won't do:
> > 1. backmerge -next to -rc
> 
> So why is this not an option? Well ok so I don't mean we should merge. I guess
> I'm more looking to cherry-pick.

Backmerge will cause to the situation where features are brought to -rc.
The cherry-pick will be:
1. Revert [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] from -next
2. Apply [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] to -rc

Thanks
Jason Gunthorpe Jan. 20, 2023, 5:42 p.m. UTC | #10
On Fri, Jan 20, 2023 at 07:09:43PM +0200, Leon Romanovsky wrote:

> Backmerge will cause to the situation where features are brought to -rc.
> The cherry-pick will be:
> 1. Revert [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] from -next
> 2. Apply [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] to -rc

You don't need to revert, we just need to merge a RC release to -next
and deal with the conflict, if any.

Jason
Dennis Dalessandro Jan. 20, 2023, 5:50 p.m. UTC | #11
On 1/20/23 12:42 PM, Jason Gunthorpe wrote:
> On Fri, Jan 20, 2023 at 07:09:43PM +0200, Leon Romanovsky wrote:
> 
>> Backmerge will cause to the situation where features are brought to -rc.
>> The cherry-pick will be:
>> 1. Revert [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] from -next
>> 2. Apply [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] to -rc
> 
> You don't need to revert, we just need to merge a RC release to -next
> and deal with the conflict, if any.

Thanks this sounds like a good way to go.

-Denny
Leon Romanovsky Jan. 22, 2023, 9:14 a.m. UTC | #12
On Fri, Jan 20, 2023 at 12:50:35PM -0500, Dennis Dalessandro wrote:
> On 1/20/23 12:42 PM, Jason Gunthorpe wrote:
> > On Fri, Jan 20, 2023 at 07:09:43PM +0200, Leon Romanovsky wrote:
> > 
> >> Backmerge will cause to the situation where features are brought to -rc.
> >> The cherry-pick will be:
> >> 1. Revert [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] from -next
> >> 2. Apply [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] to -rc
> > 
> > You don't need to revert, we just need to merge a RC release to -next
> > and deal with the conflict, if any.
> 
> Thanks this sounds like a good way to go.

You talked about broken -rc, but here wants to fix -next.
https://lore.kernel.org/all/bce1ab36-66e4-465c-e051-94e397d108ba@cornelisnetworks.com/
https://lore.kernel.org/all/Y8T5602bNhscGixb@unreal/

Thanks

> 
> -Denny
Dennis Dalessandro Jan. 23, 2023, 4:49 p.m. UTC | #13
On 1/22/23 4:14 AM, Leon Romanovsky wrote:
> On Fri, Jan 20, 2023 at 12:50:35PM -0500, Dennis Dalessandro wrote:
>> On 1/20/23 12:42 PM, Jason Gunthorpe wrote:
>>> On Fri, Jan 20, 2023 at 07:09:43PM +0200, Leon Romanovsky wrote:
>>>
>>>> Backmerge will cause to the situation where features are brought to -rc.
>>>> The cherry-pick will be:
>>>> 1. Revert [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] from -next
>>>> 2. Apply [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] to -rc
>>>
>>> You don't need to revert, we just need to merge a RC release to -next
>>> and deal with the conflict, if any.
>>
>> Thanks this sounds like a good way to go.
> 
> You talked about broken -rc, but here wants to fix -next.
> https://lore.kernel.org/all/bce1ab36-66e4-465c-e051-94e397d108ba@cornelisnetworks.com/
> https://lore.kernel.org/all/Y8T5602bNhscGixb@unreal/


> >> As a side effect of this, can we pull patch 2/7 from this series into the RC?
> >
> > No, everything is in for-rc/for-next now.
>
> Without that patch there will be a regression in 6.2.

Sorry it's not clear. Want to move or include patch to keep -rc from being
broken. Your #2 above. I'm not concerned about #1 b/c it will fix itself in time
after merging with 6.2-rc.

-Denny
Dennis Dalessandro Jan. 30, 2023, 9:57 p.m. UTC | #14
On 1/23/23 11:49 AM, Dennis Dalessandro wrote:
> On 1/22/23 4:14 AM, Leon Romanovsky wrote:
>> On Fri, Jan 20, 2023 at 12:50:35PM -0500, Dennis Dalessandro wrote:
>>> On 1/20/23 12:42 PM, Jason Gunthorpe wrote:
>>>> On Fri, Jan 20, 2023 at 07:09:43PM +0200, Leon Romanovsky wrote:
>>>>
>>>>> Backmerge will cause to the situation where features are brought to -rc.
>>>>> The cherry-pick will be:
>>>>> 1. Revert [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] from -next
>>>>> 2. Apply [PATCH for-next 2/7] IB/hfi1: Assign npages earlier] to -rc
>>>>
>>>> You don't need to revert, we just need to merge a RC release to -next
>>>> and deal with the conflict, if any.
>>>
>>> Thanks this sounds like a good way to go.
>>
>> You talked about broken -rc, but here wants to fix -next.
>> https://lore.kernel.org/all/bce1ab36-66e4-465c-e051-94e397d108ba@cornelisnetworks.com/
>> https://lore.kernel.org/all/Y8T5602bNhscGixb@unreal/
> 
> 
>>>> As a side effect of this, can we pull patch 2/7 from this series into the RC?
>>>
>>> No, everything is in for-rc/for-next now.
>>
>> Without that patch there will be a regression in 6.2.
> 
> Sorry it's not clear. Want to move or include patch to keep -rc from being
> broken. Your #2 above. I'm not concerned about #1 b/c it will fix itself in time
> after merging with 6.2-rc.

I didn't see this make it out yet. Can this still make it in for -rc? Based on
Jason's reply [1] sounds like it will just work itself out in for-next.

[1] https://lore.kernel.org/linux-rdma/Y8rSiD5s+ZFV666t@nvidia.com/

-Denny
Jason Gunthorpe Jan. 31, 2023, 2:58 p.m. UTC | #15
On Mon, Jan 30, 2023 at 04:57:56PM -0500, Dennis Dalessandro wrote:

> I didn't see this make it out yet. Can this still make it in for -rc? Based on
> Jason's reply [1] sounds like it will just work itself out in for-next.
> 
> [1] https://lore.kernel.org/linux-rdma/Y8rSiD5s+ZFV666t@nvidia.com/

Well, it looks OK to me, you should do a test merge yourself and
confirm nothing got mangled

Jason
Dennis Dalessandro Jan. 31, 2023, 4:57 p.m. UTC | #16
On 1/31/23 9:58 AM, Jason Gunthorpe wrote:
> On Mon, Jan 30, 2023 at 04:57:56PM -0500, Dennis Dalessandro wrote:
> 
>> I didn't see this make it out yet. Can this still make it in for -rc? Based on
>> Jason's reply [1] sounds like it will just work itself out in for-next.
>>
>> [1] https://lore.kernel.org/linux-rdma/Y8rSiD5s+ZFV666t@nvidia.com/
> 
> Well, it looks OK to me, you should do a test merge yourself and
> confirm nothing got mangled

I tested this last week by cherry picking:

a479433a6b7a ("IB/hfi1: Assign npages earlier")

onto for-rc, then merged for-rc into for-next. Looks ok. No conflicts.

-Denny
Leon Romanovsky Feb. 1, 2023, 8:40 a.m. UTC | #17
On Tue, Jan 31, 2023 at 11:57:22AM -0500, Dennis Dalessandro wrote:
> On 1/31/23 9:58 AM, Jason Gunthorpe wrote:
> > On Mon, Jan 30, 2023 at 04:57:56PM -0500, Dennis Dalessandro wrote:
> > 
> >> I didn't see this make it out yet. Can this still make it in for -rc? Based on
> >> Jason's reply [1] sounds like it will just work itself out in for-next.
> >>
> >> [1] https://lore.kernel.org/linux-rdma/Y8rSiD5s+ZFV666t@nvidia.com/
> > 
> > Well, it looks OK to me, you should do a test merge yourself and
> > confirm nothing got mangled
> 
> I tested this last week by cherry picking:
> 
> a479433a6b7a ("IB/hfi1: Assign npages earlier")
> 
> onto for-rc, then merged for-rc into for-next. Looks ok. No conflicts.

Jason took this patch to his wip/jgg-for-rc branch.

Thanks

> 
> -Denny
Dennis Dalessandro Feb. 2, 2023, 3:56 p.m. UTC | #18
On 2/1/23 3:40 AM, Leon Romanovsky wrote:
> On Tue, Jan 31, 2023 at 11:57:22AM -0500, Dennis Dalessandro wrote:
>> On 1/31/23 9:58 AM, Jason Gunthorpe wrote:
>>> On Mon, Jan 30, 2023 at 04:57:56PM -0500, Dennis Dalessandro wrote:
>>>
>>>> I didn't see this make it out yet. Can this still make it in for -rc? Based on
>>>> Jason's reply [1] sounds like it will just work itself out in for-next.
>>>>
>>>> [1] https://lore.kernel.org/linux-rdma/Y8rSiD5s+ZFV666t@nvidia.com/
>>>
>>> Well, it looks OK to me, you should do a test merge yourself and
>>> confirm nothing got mangled
>>
>> I tested this last week by cherry picking:
>>
>> a479433a6b7a ("IB/hfi1: Assign npages earlier")
>>
>> onto for-rc, then merged for-rc into for-next. Looks ok. No conflicts.
> 
> Jason took this patch to his wip/jgg-for-rc branch.

Ok cool. Jason if you want a fixes lines to tack onto the commit message...

Fixes:  b3deec25847b ("IB/hfi1: Remove user expected buffer invalidate race")

-Denny