Hi all,
Today's linux-next merge of the block tree got a conflict in:
drivers/nvme/target/rdma.c
between commit:
5733111dcd97 ("nvmet-rdma: use new shared CQ mechanism")
from the rdma tree and commits:
b0012dd39715 ("nvmet-rdma: use SRQ per completion vector")
b09160c3996c ("nvmet-rdma: add metadata/T10-PI support")
from the block tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/nvme/target/rdma.c
index 2405db8bd855,d5141780592e..000000000000
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@@ -589,7 -751,8 +752,8 @@@ static void nvmet_rdma_read_data_done(s
{
struct nvmet_rdma_rsp *rsp =
container_of(wc->wr_cqe, struct nvmet_rdma_rsp, read_cqe);
- struct nvmet_rdma_queue *queue = cq->cq_context;
+ struct nvmet_rdma_queue *queue = wc->qp->qp_context;
+ u16 status = 0;
WARN_ON(rsp->n_rdma <= 0);
atomic_add(rsp->n_rdma, &queue->sq_wr_avail);
@@@ -996,8 -1257,9 +1258,8 @@@ static int nvmet_rdma_create_queue_ib(s
*/
nr_cqe = queue->recv_queue_size + 2 * queue->send_queue_size;
- queue->cq = ib_cq_pool_get(ndev->device, nr_cqe + 1, comp_vector,
- queue->cq = ib_alloc_cq(ndev->device, queue,
- nr_cqe + 1, queue->comp_vector,
- IB_POLL_WORKQUEUE);
++ queue->cq = ib_cq_pool_get(ndev->device, nr_cqe + 1, queue->comp_vector,
+ IB_POLL_WORKQUEUE);
if (IS_ERR(queue->cq)) {
ret = PTR_ERR(queue->cq);
pr_err("failed to create CQ cqe= %d ret= %d\n",
On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
> Hi all,
Hi,
This looks good to me.
Can you share a pointer to the tree so we'll test it in our labs ?
need to re-test:
1. srq per core
2. srq per core + T10-PI
And both will run with shared CQ.
>
> Today's linux-next merge of the block tree got a conflict in:
>
> drivers/nvme/target/rdma.c
>
> between commit:
>
> 5733111dcd97 ("nvmet-rdma: use new shared CQ mechanism")
>
> from the rdma tree and commits:
>
> b0012dd39715 ("nvmet-rdma: use SRQ per completion vector")
> b09160c3996c ("nvmet-rdma: add metadata/T10-PI support")
>
> from the block tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
Hi Max,
On Tue, 2 Jun 2020 11:37:26 +0300 Max Gurtovoy <[email protected]> wrote:
>
> On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
>
> This looks good to me.
>
> Can you share a pointer to the tree so we'll test it in our labs ?
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
you want tag next-20200602
or if you just want that trees that conflicted, then
block: git://git.kernel.dk/linux-block.git branch for-next
rdma: git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git branch for-next
--
Cheers,
Stephen Rothwell
On 6/2/20 1:01 PM, Jason Gunthorpe wrote:
> On Tue, Jun 02, 2020 at 11:37:26AM +0300, Max Gurtovoy wrote:
>>
>> On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
>>> Hi all,
>>
>> Hi,
>>
>> This looks good to me.
>>
>> Can you share a pointer to the tree so we'll test it in our labs ?
>>
>> need to re-test:
>>
>> 1. srq per core
>>
>> 2. srq per core + T10-PI
>>
>> And both will run with shared CQ.
>
> Max, this is too much conflict to send to Linus between your own
> patches. I am going to drop the nvme part of this from RDMA.
>
> Normally I don't like applying partial series, but due to this tree
> split, you can send the rebased nvme part through the nvme/block tree
> at rc1 in two weeks..
Was going to comment that this is probably how it should have been
done to begin with. If we have multiple conflicts like that between
two trees, someone is doing something wrong...
--
Jens Axboe
On Tue, Jun 02, 2020 at 11:37:26AM +0300, Max Gurtovoy wrote:
>
> On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
> > Hi all,
>
> Hi,
>
> This looks good to me.
>
> Can you share a pointer to the tree so we'll test it in our labs ?
>
> need to re-test:
>
> 1. srq per core
>
> 2. srq per core + T10-PI
>
> And both will run with shared CQ.
Max, this is too much conflict to send to Linus between your own
patches. I am going to drop the nvme part of this from RDMA.
Normally I don't like applying partial series, but due to this tree
split, you can send the rebased nvme part through the nvme/block tree
at rc1 in two weeks..
Jason
On Tue, Jun 02, 2020 at 01:02:55PM -0600, Jens Axboe wrote:
> On 6/2/20 1:01 PM, Jason Gunthorpe wrote:
> > On Tue, Jun 02, 2020 at 11:37:26AM +0300, Max Gurtovoy wrote:
> >>
> >> On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
> >>> Hi all,
> >>
> >> Hi,
> >>
> >> This looks good to me.
> >>
> >> Can you share a pointer to the tree so we'll test it in our labs ?
> >>
> >> need to re-test:
> >>
> >> 1. srq per core
> >>
> >> 2. srq per core + T10-PI
> >>
> >> And both will run with shared CQ.
> >
> > Max, this is too much conflict to send to Linus between your own
> > patches. I am going to drop the nvme part of this from RDMA.
> >
> > Normally I don't like applying partial series, but due to this tree
> > split, you can send the rebased nvme part through the nvme/block tree
> > at rc1 in two weeks..
>
> Was going to comment that this is probably how it should have been
> done to begin with. If we have multiple conflicts like that between
> two trees, someone is doing something wrong...
Well, on the other hand having people add APIs in one tree and then
(promised) consumers in another tree later on has proven problematic
in the past. It is best to try to avoid that, but in this case I don't
think Max will have any delay to get the API consumer into nvme in two
weeks.
Jason
On 6/2/20 1:09 PM, Jason Gunthorpe wrote:
> On Tue, Jun 02, 2020 at 01:02:55PM -0600, Jens Axboe wrote:
>> On 6/2/20 1:01 PM, Jason Gunthorpe wrote:
>>> On Tue, Jun 02, 2020 at 11:37:26AM +0300, Max Gurtovoy wrote:
>>>>
>>>> On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
>>>>> Hi all,
>>>>
>>>> Hi,
>>>>
>>>> This looks good to me.
>>>>
>>>> Can you share a pointer to the tree so we'll test it in our labs ?
>>>>
>>>> need to re-test:
>>>>
>>>> 1. srq per core
>>>>
>>>> 2. srq per core + T10-PI
>>>>
>>>> And both will run with shared CQ.
>>>
>>> Max, this is too much conflict to send to Linus between your own
>>> patches. I am going to drop the nvme part of this from RDMA.
>>>
>>> Normally I don't like applying partial series, but due to this tree
>>> split, you can send the rebased nvme part through the nvme/block tree
>>> at rc1 in two weeks..
>>
>> Was going to comment that this is probably how it should have been
>> done to begin with. If we have multiple conflicts like that between
>> two trees, someone is doing something wrong...
>
> Well, on the other hand having people add APIs in one tree and then
> (promised) consumers in another tree later on has proven problematic
> in the past. It is best to try to avoid that, but in this case I don't
> think Max will have any delay to get the API consumer into nvme in two
> weeks.
Having conflicting trees is a problem. If there's a dependency for
two trees for some new work, then just have a separate branch that's
built on those two. For NVMe core work, then it should include the
pending NVMe changes.
--
Jens Axboe
On 6/3/2020 12:37 AM, Jens Axboe wrote:
> On 6/2/20 1:09 PM, Jason Gunthorpe wrote:
>> On Tue, Jun 02, 2020 at 01:02:55PM -0600, Jens Axboe wrote:
>>> On 6/2/20 1:01 PM, Jason Gunthorpe wrote:
>>>> On Tue, Jun 02, 2020 at 11:37:26AM +0300, Max Gurtovoy wrote:
>>>>> On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
>>>>>> Hi all,
>>>>> Hi,
>>>>>
>>>>> This looks good to me.
>>>>>
>>>>> Can you share a pointer to the tree so we'll test it in our labs ?
>>>>>
>>>>> need to re-test:
>>>>>
>>>>> 1. srq per core
>>>>>
>>>>> 2. srq per core + T10-PI
>>>>>
>>>>> And both will run with shared CQ.
>>>> Max, this is too much conflict to send to Linus between your own
>>>> patches. I am going to drop the nvme part of this from RDMA.
>>>>
>>>> Normally I don't like applying partial series, but due to this tree
>>>> split, you can send the rebased nvme part through the nvme/block tree
>>>> at rc1 in two weeks..
Yes, I'll send it in 2 weeks.
Actually I hoped the iSER patches for CQ pool will be sent in this
series but eventually they were not.
This way we could have taken only the iser part and the new API.
I saw the pulled version too late since I wasn't CCed to it and it was
already merged before I had a chance to warn you about possible conflict.
I think in general we should try to add new RDMA APIs first with
iSER/SRP and avoid conflicting trees.
>>> Was going to comment that this is probably how it should have been
>>> done to begin with. If we have multiple conflicts like that between
>>> two trees, someone is doing something wrong...
>> Well, on the other hand having people add APIs in one tree and then
>> (promised) consumers in another tree later on has proven problematic
>> in the past. It is best to try to avoid that, but in this case I don't
>> think Max will have any delay to get the API consumer into nvme in two
>> weeks.
> Having conflicting trees is a problem. If there's a dependency for
> two trees for some new work, then just have a separate branch that's
> built on those two. For NVMe core work, then it should include the
> pending NVMe changes.
I guess it's hard to do so during merge window since the block and rdma
trees are not in sync.
I think it would have been a good idea to add Jens to CC and mention
that we're posting code that is maintained by 2 different trees in the
cover latter.
On Wed, Jun 03, 2020 at 01:40:51AM +0300, Max Gurtovoy wrote:
>
> On 6/3/2020 12:37 AM, Jens Axboe wrote:
> > On 6/2/20 1:09 PM, Jason Gunthorpe wrote:
> > > On Tue, Jun 02, 2020 at 01:02:55PM -0600, Jens Axboe wrote:
> > > > On 6/2/20 1:01 PM, Jason Gunthorpe wrote:
> > > > > On Tue, Jun 02, 2020 at 11:37:26AM +0300, Max Gurtovoy wrote:
> > > > > > On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
> > > > > > > Hi all,
> > > > > > Hi,
> > > > > >
> > > > > > This looks good to me.
> > > > > >
> > > > > > Can you share a pointer to the tree so we'll test it in our labs ?
> > > > > >
> > > > > > need to re-test:
> > > > > >
> > > > > > 1. srq per core
> > > > > >
> > > > > > 2. srq per core + T10-PI
> > > > > >
> > > > > > And both will run with shared CQ.
> > > > > Max, this is too much conflict to send to Linus between your own
> > > > > patches. I am going to drop the nvme part of this from RDMA.
> > > > >
> > > > > Normally I don't like applying partial series, but due to this tree
> > > > > split, you can send the rebased nvme part through the nvme/block tree
> > > > > at rc1 in two weeks..
>
> Yes, I'll send it in 2 weeks.
>
> Actually I hoped the iSER patches for CQ pool will be sent in this series
> but eventually they were not.
>
> This way we could have taken only the iser part and the new API.
>
> I saw the pulled version too late since I wasn't CCed to it and it was
> already merged before I had a chance to warn you about possible conflict.
>
> I think in general we should try to add new RDMA APIs first with iSER/SRP
> and avoid conflicting trees.
If you are careful we can construct a shared branch and if Jens/etc is
willing he can pull the RDMA base code after RDMA merges the branch
and then apply the nvme parts. This is how things work with netdev
It is tricky and you have to plan for it during your submission step,
but we should be able to manage in most cases if this comes up more
often.
Jason
On 6/3/2020 2:32 AM, Jason Gunthorpe wrote:
> On Wed, Jun 03, 2020 at 01:40:51AM +0300, Max Gurtovoy wrote:
>> On 6/3/2020 12:37 AM, Jens Axboe wrote:
>>> On 6/2/20 1:09 PM, Jason Gunthorpe wrote:
>>>> On Tue, Jun 02, 2020 at 01:02:55PM -0600, Jens Axboe wrote:
>>>>> On 6/2/20 1:01 PM, Jason Gunthorpe wrote:
>>>>>> On Tue, Jun 02, 2020 at 11:37:26AM +0300, Max Gurtovoy wrote:
>>>>>>> On 6/2/2020 5:56 AM, Stephen Rothwell wrote:
>>>>>>>> Hi all,
>>>>>>> Hi,
>>>>>>>
>>>>>>> This looks good to me.
>>>>>>>
>>>>>>> Can you share a pointer to the tree so we'll test it in our labs ?
>>>>>>>
>>>>>>> need to re-test:
>>>>>>>
>>>>>>> 1. srq per core
>>>>>>>
>>>>>>> 2. srq per core + T10-PI
>>>>>>>
>>>>>>> And both will run with shared CQ.
>>>>>> Max, this is too much conflict to send to Linus between your own
>>>>>> patches. I am going to drop the nvme part of this from RDMA.
>>>>>>
>>>>>> Normally I don't like applying partial series, but due to this tree
>>>>>> split, you can send the rebased nvme part through the nvme/block tree
>>>>>> at rc1 in two weeks..
>> Yes, I'll send it in 2 weeks.
>>
>> Actually I hoped the iSER patches for CQ pool will be sent in this series
>> but eventually they were not.
>>
>> This way we could have taken only the iser part and the new API.
>>
>> I saw the pulled version too late since I wasn't CCed to it and it was
>> already merged before I had a chance to warn you about possible conflict.
>>
>> I think in general we should try to add new RDMA APIs first with iSER/SRP
>> and avoid conflicting trees.
> If you are careful we can construct a shared branch and if Jens/etc is
> willing he can pull the RDMA base code after RDMA merges the branch
> and then apply the nvme parts. This is how things work with netdev
>
> It is tricky and you have to plan for it during your submission step,
> but we should be able to manage in most cases if this comes up more
> often.
I think we can construct a branch like this for dedicated series and
delete it after the acceptance.
In case of new APIs for RDMA that involve touching NVMe stuff - we'll
create this branch and ask Jens to pull it as you suggested.
And as a general note, I suggest we won't merge NVMe/RDMA stuff to
rdma-next without cooperation with Jens.
-Max.
>
> Jason