[PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Wei Hu (Xavier)]

This patch-set introduces three patches to fix about the memory
related issues.

Wei Hu (Xavier) (3):
  RDMA/hns: Fix the issue of IOVA not page continuous in hip08
  RDMA/hns: Get rid of virt_to_page and vmap calls after
    dma_alloc_coherent
  RDMA/hns: Get rid of page operation after dma_alloc_coherent

 drivers/infiniband/hw/hns/hns_roce_alloc.c  | 23 -----------------------
 drivers/infiniband/hw/hns/hns_roce_device.h |  4 +---
 drivers/infiniband/hw/hns/hns_roce_hem.c    | 25 +++++++++++++------------
 drivers/infiniband/hw/hns/hns_roce_hem.h    |  1 +
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 22 +++++++++++++++-------
 5 files changed, 30 insertions(+), 45 deletions(-)

-- 
1.9.1

[PATCH 1/3] RDMA/hns: Fix the issue of IOVA not page continuous in hip08

[Wei Hu (Xavier)]

If the smmu is enabled, the length of sg obtained from
__iommu_map_sg_attrs is not 4kB. When the IOVA is set with the sg
dma address, the IOVA will not be page continuous.
In order to fix this issue, the IOVA should be calculated based on
the sg length.

Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Yixian Liu <liuyixian@huawei.com>
Signed-off-by: Xiping Zhang (Francis) <zhangxiping3@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 04281d0..4d3e976 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -1131,9 +1131,11 @@ static int hns_roce_v2_write_mtpt(void *mb_buf, struct hns_roce_mr *mr,
 {
 	struct hns_roce_v2_mpt_entry *mpt_entry;
 	struct scatterlist *sg;
+	u64 page_addr;
 	u64 *pages;
+	int i, j;
+	int len;
 	int entry;
-	int i;
 
 	mpt_entry = mb_buf;
 	memset(mpt_entry, 0, sizeof(*mpt_entry));
@@ -1191,14 +1193,20 @@ static int hns_roce_v2_write_mtpt(void *mb_buf, struct hns_roce_mr *mr,
 
 	i = 0;
 	for_each_sg(mr->umem->sg_head.sgl, sg, mr->umem->nmap, entry) {
-		pages[i] = ((u64)sg_dma_address(sg)) >> 6;
-
-		/* Record the first 2 entry directly to MTPT table */
-		if (i >= HNS_ROCE_V2_MAX_INNER_MTPT_NUM - 1)
-			break;
-		i++;
+		len = sg_dma_len(sg) >> PAGE_SHIFT;
+		for (j = 0; j < len; ++j) {
+			page_addr = sg_dma_address(sg) +
+				    (j << mr->umem->page_shift);
+			pages[i] = page_addr >> 6;
+
+			/* Record the first 2 entry directly to MTPT table */
+			if (i >= HNS_ROCE_V2_MAX_INNER_MTPT_NUM - 1)
+				goto found;
+			i++;
+		}
 	}
 
+found:
 	mpt_entry->pa0_l = cpu_to_le32(lower_32_bits(pages[0]));
 	roce_set_field(mpt_entry->byte_56_pa0_h, V2_MPT_BYTE_56_PA0_H_M,
 		       V2_MPT_BYTE_56_PA0_H_S,
-- 
1.9.1

[PATCH 2/3] RDMA/hns: Get rid of virt_to_page and vmap calls after dma_alloc_coherent

[Wei Hu (Xavier)]

In general dma_alloc_coherent() return a CPU virtual address and
a DMA address, and we have no guarantee that the virtual address
is either in the linear map or vmalloc, and not some other special
place. we have no guarantee that the underlying memory even has
an associated struct page at all.

In current code, there are incorrect usage as below:
dma_alloc_coherent + virt_to_page + vmap. This patch fixes it to
get rid of virt_to_page and vmap calls.

Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Yixian Liu <liuyixian@huawei.com>
Signed-off-by: Xiping Zhang (Francis) <zhangxiping3@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_alloc.c  | 23 -----------------------
 drivers/infiniband/hw/hns/hns_roce_device.h |  4 +---
 2 files changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_alloc.c b/drivers/infiniband/hw/hns/hns_roce_alloc.c
index 3e4c525..a40ec93 100644
--- a/drivers/infiniband/hw/hns/hns_roce_alloc.c
+++ b/drivers/infiniband/hw/hns/hns_roce_alloc.c
@@ -162,14 +162,10 @@ void hns_roce_buf_free(struct hns_roce_dev *hr_dev, u32 size,
 {
 	int i;
 	struct device *dev = hr_dev->dev;
-	u32 bits_per_long = BITS_PER_LONG;
 
 	if (buf->nbufs == 1) {
 		dma_free_coherent(dev, size, buf->direct.buf, buf->direct.map);
 	} else {
-		if (bits_per_long == 64 && buf->page_shift == PAGE_SHIFT)
-			vunmap(buf->direct.buf);
-
 		for (i = 0; i < buf->nbufs; ++i)
 			if (buf->page_list[i].buf)
 				dma_free_coherent(dev, 1 << buf->page_shift,
@@ -185,9 +181,7 @@ int hns_roce_buf_alloc(struct hns_roce_dev *hr_dev, u32 size, u32 max_direct,
 {
 	int i = 0;
 	dma_addr_t t;
-	struct page **pages;
 	struct device *dev = hr_dev->dev;
-	u32 bits_per_long = BITS_PER_LONG;
 	u32 page_size = 1 << page_shift;
 	u32 order;
 
@@ -236,23 +230,6 @@ int hns_roce_buf_alloc(struct hns_roce_dev *hr_dev, u32 size, u32 max_direct,
 			buf->page_list[i].map = t;
 			memset(buf->page_list[i].buf, 0, page_size);
 		}
-		if (bits_per_long == 64 && page_shift == PAGE_SHIFT) {
-			pages = kmalloc_array(buf->nbufs, sizeof(*pages),
-					      GFP_KERNEL);
-			if (!pages)
-				goto err_free;
-
-			for (i = 0; i < buf->nbufs; ++i)
-				pages[i] = virt_to_page(buf->page_list[i].buf);
-
-			buf->direct.buf = vmap(pages, buf->nbufs, VM_MAP,
-					       PAGE_KERNEL);
-			kfree(pages);
-			if (!buf->direct.buf)
-				goto err_free;
-		} else {
-			buf->direct.buf = NULL;
-		}
 	}
 
 	return 0;
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index dde5178..dcfd209 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -795,11 +795,9 @@ static inline void hns_roce_write64_k(__be32 val[2], void __iomem *dest)
 
 static inline void *hns_roce_buf_offset(struct hns_roce_buf *buf, int offset)
 {
-	u32 bits_per_long_val = BITS_PER_LONG;
 	u32 page_size = 1 << buf->page_shift;
 
-	if ((bits_per_long_val == 64 && buf->page_shift == PAGE_SHIFT) ||
-	    buf->nbufs == 1)
+	if (buf->nbufs == 1)
 		return (char *)(buf->direct.buf) + offset;
 	else
 		return (char *)(buf->page_list[offset >> buf->page_shift].buf) +
-- 
1.9.1

[PATCH 3/3] RDMA/hns: Get rid of page operation after dma_alloc_coherent

[Wei Hu (Xavier)]

In general dma_alloc_coherent() return a CPU virtual address and
a DMA address, and we have no guarantee that the underlying memory
even has an associated struct page at all.

This patch gets rid of the page operation after dma_alloc_coherent,
and records the VA returned form dma_alloc_coherent in the struct
of hem.

Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Yixian Liu <liuyixian@huawei.com>
Signed-off-by: Xiping Zhang (Francis) <zhangxiping3@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hem.c | 25 +++++++++++++------------
 drivers/infiniband/hw/hns/hns_roce_hem.h |  1 +
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
index 8b733a6..0eeabfb 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
@@ -224,6 +224,7 @@ static struct hns_roce_hem *hns_roce_alloc_hem(struct hns_roce_dev *hr_dev,
 			sg_init_table(chunk->mem, HNS_ROCE_HEM_CHUNK_LEN);
 			chunk->npages = 0;
 			chunk->nsg = 0;
+			memset(chunk->buf, 0, sizeof(chunk->buf));
 			list_add_tail(&chunk->list, &hem->chunk_list);
 		}
 
@@ -240,8 +241,7 @@ static struct hns_roce_hem *hns_roce_alloc_hem(struct hns_roce_dev *hr_dev,
 		if (!buf)
 			goto fail;
 
-		sg_set_buf(mem, buf, PAGE_SIZE << order);
-		WARN_ON(mem->offset);
+		chunk->buf[chunk->npages] = buf;
 		sg_dma_len(mem) = PAGE_SIZE << order;
 
 		++chunk->npages;
@@ -267,8 +267,8 @@ void hns_roce_free_hem(struct hns_roce_dev *hr_dev, struct hns_roce_hem *hem)
 	list_for_each_entry_safe(chunk, tmp, &hem->chunk_list, list) {
 		for (i = 0; i < chunk->npages; ++i)
 			dma_free_coherent(hr_dev->dev,
-				   chunk->mem[i].length,
-				   lowmem_page_address(sg_page(&chunk->mem[i])),
+				   sg_dma_len(&chunk->mem[i]),
+				   chunk->buf[i],
 				   sg_dma_address(&chunk->mem[i]));
 		kfree(chunk);
 	}
@@ -722,11 +722,12 @@ void *hns_roce_table_find(struct hns_roce_dev *hr_dev,
 	struct hns_roce_hem_chunk *chunk;
 	struct hns_roce_hem_mhop mhop;
 	struct hns_roce_hem *hem;
-	struct page *page = NULL;
+	void *addr = NULL;
 	unsigned long mhop_obj = obj;
 	unsigned long obj_per_chunk;
 	unsigned long idx_offset;
 	int offset, dma_offset;
+	int length;
 	int i, j;
 	u32 hem_idx = 0;
 
@@ -763,25 +764,25 @@ void *hns_roce_table_find(struct hns_roce_dev *hr_dev,
 
 	list_for_each_entry(chunk, &hem->chunk_list, list) {
 		for (i = 0; i < chunk->npages; ++i) {
+			length = sg_dma_len(&chunk->mem[i]);
 			if (dma_handle && dma_offset >= 0) {
-				if (sg_dma_len(&chunk->mem[i]) >
-				    (u32)dma_offset)
+				if (length > (u32)dma_offset)
 					*dma_handle = sg_dma_address(
 						&chunk->mem[i]) + dma_offset;
-				dma_offset -= sg_dma_len(&chunk->mem[i]);
+				dma_offset -= length;
 			}
 
-			if (chunk->mem[i].length > (u32)offset) {
-				page = sg_page(&chunk->mem[i]);
+			if (length > (u32)offset) {
+				addr = chunk->buf[i] + offset;
 				goto out;
 			}
-			offset -= chunk->mem[i].length;
+			offset -= length;
 		}
 	}
 
 out:
 	mutex_unlock(&table->mutex);
-	return page ? lowmem_page_address(page) + offset : NULL;
+	return addr;
 }
 EXPORT_SYMBOL_GPL(hns_roce_table_find);
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.h b/drivers/infiniband/hw/hns/hns_roce_hem.h
index db66db1..e8850d5 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.h
@@ -78,6 +78,7 @@ struct hns_roce_hem_chunk {
 	int			 npages;
 	int			 nsg;
 	struct scatterlist	 mem[HNS_ROCE_HEM_CHUNK_LEN];
+	void			 *buf[HNS_ROCE_HEM_CHUNK_LEN];
 };
 
 struct hns_roce_hem {
-- 
1.9.1

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Jason Gunthorpe]

On Mon, Nov 27, 2017 at 10:41:21AM +0800, Wei Hu (Xavier) wrote:
> This patch-set introduces three patches to fix about the memory
> related issues.

If these patches are for-rc can you revise the commit message to more
clearly explain what the bug is, and supply Fixes lines?

Otherwise these will go to 4.16

It looks like the first one is for-rc material as it looks like DMA
will fail sometimes if the smmu is enabled?

The other two are less clear to me - is this just fixing the code to
conform to the DMA API, or is there an actual real user-impacting bug here?

Thanks
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On Mon, Nov 27, 2017 at 11:36:37AM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 27, 2017 at 10:41:21AM +0800, Wei Hu (Xavier) wrote:
> > This patch-set introduces three patches to fix about the memory
> > related issues.
>
> If these patches are for-rc can you revise the commit message to more
> clearly explain what the bug is, and supply Fixes lines?
>
> Otherwise these will go to 4.16

Jason,

If you treat all unmarked patches (without mentioning in cover letter or
subject title) as targeted to for-next, it will make your life much more
easier than trying to pick each patch alone. As an outcome, it will make
the patch flow more predictable for us.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Wei Hu (Xavier)]

On 2017/11/28 2:36, Jason Gunthorpe wrote:
> On Mon, Nov 27, 2017 at 10:41:21AM +0800, Wei Hu (Xavier) wrote:
>> This patch-set introduces three patches to fix about the memory
>> related issues.
> If these patches are for-rc can you revise the commit message to more
> clearly explain what the bug is, and supply Fixes lines?
Ok,  I will modify the commit message and add Fixes lines.
And I will send patch V2 for-rc.
Thanks.

> Otherwise these will go to 4.16
>
> It looks like the first one is for-rc material as it looks like DMA
> will fail sometimes if the smmu is enabled?
Yes, The first one fixes the MTPT configuration error that probably
cause dma operation
    failure.
> The other two are less clear to me - is this just fixing the code to
> conform to the DMA API, or is there an actual real user-impacting bug here?
The other two patches fixes incorrect usage of dma API that may
cause coherency problems.

Regards
Wei Hu
> Thanks
> Jason
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Dennis Dalessandro]

On 11/28/2017 1:15 AM, Leon Romanovsky wrote:
> On Mon, Nov 27, 2017 at 11:36:37AM -0700, Jason Gunthorpe wrote:
>> On Mon, Nov 27, 2017 at 10:41:21AM +0800, Wei Hu (Xavier) wrote:
>>> This patch-set introduces three patches to fix about the memory
>>> related issues.
>>
>> If these patches are for-rc can you revise the commit message to more
>> clearly explain what the bug is, and supply Fixes lines?
>>
>> Otherwise these will go to 4.16
> 
> Jason,
> 
> If you treat all unmarked patches (without mentioning in cover letter or
> subject title) as targeted to for-next, it will make your life much more
> easier than trying to pick each patch alone. As an outcome, it will make
> the patch flow more predictable for us.
> 
> Thanks
> 

Agree with Leon here. Unless otherwise stated, I would think for-next 
should always be the default target. I try to tag my subject with 
for-next or for-rc to make it clear.

Now of course that being said here is an exception. I sent a series 2 
weeks ago that didn't make Doug's pull request for 4.15. I'd like to see 
some of that stuff land in the rc if there are no objections. The driver 
changes at least, the CM/SA can probably push off to for-next.

I could resubmit just the series, or you could just pick the 4 driver 
patches from patchworks whatever is easiest.

-Denny

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Jason Gunthorpe]

On Tue, Nov 28, 2017 at 08:20:09AM -0500, Dennis Dalessandro wrote:
> >
> >If you treat all unmarked patches (without mentioning in cover letter or
> >subject title) as targeted to for-next, it will make your life much more
> >easier than trying to pick each patch alone. As an outcome, it will make
> >the patch flow more predictable for us.

> 
> Agree with Leon here. Unless otherwise stated, I would think for-next should
> always be the default target. I try to tag my subject with for-next or
> for-rc to make it clear.

For Leon and Dennis I know they mark things, but other people seem to
be sending bug fixes that are rc material without any firm indication
one way or the other. I read the patches and that is my main question.

Of course if they do not come in RC acceptable format then they wait
in patchworks until -next is opened.

> Now of course that being said here is an exception. I sent a series 2 weeks
> ago that didn't make Doug's pull request for 4.15. I'd like to see some of
> that stuff land in the rc if there are no objections. The driver changes at
> least, the CM/SA can probably push off to for-next.
 
> I could resubmit just the series, or you could just pick the 4 driver
> patches from patchworks whatever is easiest.

I marked them in patchworks, but can you review the commit messages
and make sure you think Linus will see them as rc material too?

Thanks,
Jason

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Bart Van Assche]

On Tue, 2017-11-28 at 11:39 -0700, Jason Gunthorpe wrote:
> On Tue, Nov 28, 2017 at 08:20:09AM -0500, Dennis Dalessandro wrote:
> > I could resubmit just the series, or you could just pick the 4 driver
> > patches from patchworks whatever is easiest.
> 
> I marked them in patchworks, but can you review the commit messages
> and make sure you think Linus will see them as rc material too?

My understanding is that the rc stage is intended primarily for fixes for
bugs introduced during the merge window. For all patches that do not fix
bugs introduced during the merge window it should be evaluated carefully
whether or not these are important enough to be included in a rc pull request.

Bart.

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On Tue, Nov 28, 2017 at 06:50:44PM +0000, Bart Van Assche wrote:
> On Tue, 2017-11-28 at 11:39 -0700, Jason Gunthorpe wrote:
> > On Tue, Nov 28, 2017 at 08:20:09AM -0500, Dennis Dalessandro wrote:
> > > I could resubmit just the series, or you could just pick the 4 driver
> > > patches from patchworks whatever is easiest.
> >
> > I marked them in patchworks, but can you review the commit messages
> > and make sure you think Linus will see them as rc material too?
>
> My understanding is that the rc stage is intended primarily for fixes for
> bugs introduced during the merge window. For all patches that do not fix
> bugs introduced during the merge window it should be evaluated carefully
> whether or not these are important enough to be included in a rc pull request.

I had the same feeling in the previous cycles.

Thanks

>
> Bart.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Jason Gunthorpe]

On Tue, Nov 28, 2017 at 06:50:44PM +0000, Bart Van Assche wrote:
> On Tue, 2017-11-28 at 11:39 -0700, Jason Gunthorpe wrote:
> > On Tue, Nov 28, 2017 at 08:20:09AM -0500, Dennis Dalessandro wrote:
> > > I could resubmit just the series, or you could just pick the 4 driver
> > > patches from patchworks whatever is easiest.
> > 
> > I marked them in patchworks, but can you review the commit messages
> > and make sure you think Linus will see them as rc material too?
> 
> My understanding is that the rc stage is intended primarily for fixes for
> bugs introduced during the merge window. For all patches that do not fix
> bugs introduced during the merge window it should be evaluated carefully
> whether or not these are important enough to be included in a rc pull request.

Oh? I thought it was more up to the maintainer discretion within some
limits. Eg I thought we were running rdma more with the 'if it is OK
for -stable, then it is OK -rc' kind of mentality?

eg -rc is for stablization and bugfixing only, and not restricted only
to bugs introduced in the latest merge window??

Have you seen Linus give hard guidance on this topic? I looked for a
bit and found nothing.

I will sync with Doug on this and confirm..

Jason

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Dennis Dalessandro]

On 11/28/2017 2:33 PM, Jason Gunthorpe wrote:
> On Tue, Nov 28, 2017 at 06:50:44PM +0000, Bart Van Assche wrote:
>> On Tue, 2017-11-28 at 11:39 -0700, Jason Gunthorpe wrote:
>>> On Tue, Nov 28, 2017 at 08:20:09AM -0500, Dennis Dalessandro wrote:
>>>> I could resubmit just the series, or you could just pick the 4 driver
>>>> patches from patchworks whatever is easiest.
>>>
>>> I marked them in patchworks, but can you review the commit messages
>>> and make sure you think Linus will see them as rc material too?
>>
>> My understanding is that the rc stage is intended primarily for fixes for
>> bugs introduced during the merge window. For all patches that do not fix
>> bugs introduced during the merge window it should be evaluated carefully
>> whether or not these are important enough to be included in a rc pull request.
> 
> Oh? I thought it was more up to the maintainer discretion within some
> limits. Eg I thought we were running rdma more with the 'if it is OK
> for -stable, then it is OK -rc' kind of mentality?

That's always been my understanding.

> eg -rc is for stablization and bugfixing only, and not restricted only
> to bugs introduced in the latest merge window??

Makes no sense to me to have a hard restriction here. Security fixes, 
panics, and other serious bugs should certainly qualify. Of course that 
is subject to how complex the fix is.

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] RDMA/hns: Bug fixes in hns RoCE driver

[Jonathan Cameron]

On Tue, 28 Nov 2017 15:27:23 -0500
Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> On 11/28/2017 2:33 PM, Jason Gunthorpe wrote:
> > On Tue, Nov 28, 2017 at 06:50:44PM +0000, Bart Van Assche wrote:  
> >> On Tue, 2017-11-28 at 11:39 -0700, Jason Gunthorpe wrote:  
> >>> On Tue, Nov 28, 2017 at 08:20:09AM -0500, Dennis Dalessandro wrote:  
> >>>> I could resubmit just the series, or you could just pick the 4 driver
> >>>> patches from patchworks whatever is easiest.  
> >>>
> >>> I marked them in patchworks, but can you review the commit messages
> >>> and make sure you think Linus will see them as rc material too?  
> >>
> >> My understanding is that the rc stage is intended primarily for fixes for
> >> bugs introduced during the merge window. For all patches that do not fix
> >> bugs introduced during the merge window it should be evaluated carefully
> >> whether or not these are important enough to be included in a rc pull request.  
> > 
> > Oh? I thought it was more up to the maintainer discretion within some
> > limits. Eg I thought we were running rdma more with the 'if it is OK
> > for -stable, then it is OK -rc' kind of mentality?  
> 
> That's always been my understanding.

Where I maintain, I've always taken a risk judgment. If it a simple fix
for something that is obviously wrong and comes reasonably early in the
RCs I'll take it for the RCs even if the issue wasn't introduced this cycle.
There is some bias in favor of taking regression fixes even if the issue
was introduced a cycle or two back..

Something that has always been broken or broken for a long time probably
isn't effecting lots of people and can wait a few months.

Later in the RCs where there is less time to revert / follow up fix
I get fussier and will queue for the merge window even when marking
for stable.

Another obvious factor is fixes for new drivers are easier to take,
however invasive and dangerous the changes, as there can't be any
existing users to cause regressions for!

So definitely a maintainer judgment call.  I've struck this balance based
on Greg KH pushing back on my pull requests from time to time ;)

Just put on your 'grumpy maintainer' hat and base it on likelihood of
shouting and how much you care about who is doing that shouting!

> 
> > eg -rc is for stablization and bugfixing only, and not restricted only
> > to bugs introduced in the latest merge window??  
> 
> Makes no sense to me to have a hard restriction here. Security fixes, 
> panics, and other serious bugs should certainly qualify. Of course that 
> is subject to how complex the fix is.

Agreed entirely. Answer isn't always obvious though.

Jonathan

> 
> -Denny
> _______________________________________________
> linuxarm mailing list
> linuxarm-hv44wF8Li93QT0dZR+AlfA@public.gmane.org
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html