RE: [PATCH] [net-next] IB/hfi1: removed shadowed 'err' variable

2019-07-10 Thread Marciniszyn, Mike 
> Subject: [PATCH] [net-next] IB/hfi1: removed shadowed 'err' variable
> 
> I can't think of any reason for the inner variable declaration, so
> remove it to avoid the issue.
> 

I agree!

> Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory")

Thanks for catching this!

Acked-by: Mike Marciniszyn

RE: [PATCH -next] IB/qib: Add missing err handle for qib_user_sdma_rb_insert

2019-01-04 Thread Marciniszyn, Mike 
> diff --git a/drivers/infiniband/hw/qib/qib_user_sdma.c
> b/drivers/infiniband/hw/qib/qib_user_sdma.c
> index 31c523b..e87c0a7 100644
> --- a/drivers/infiniband/hw/qib/qib_user_sdma.c
> +++ b/drivers/infiniband/hw/qib/qib_user_sdma.c
> @@ -237,6 +237,8 @@ qib_user_sdma_queue_create(struct device *dev,
> int unit, int ctxt, int sctxt)
> 
>   ret = qib_user_sdma_rb_insert(_user_sdma_rb_root,
>   sdma_rb_node);
> + if (ret == 0)
> + goto err_rb;
>   }
>   pq->sdma_rb_node = sdma_rb_node;
> 
> --
> 2.7.0
> 

Thanks!

This patch is ok.

The NULL returned from this routine will result in an error return up to PSM 
which will fail.

The bail branch will do the necessary cleanup, including freeing the node that 
couldn't be added.

Acked-by: Mike Marciniszyn

RE: [PATCH] IB/qib: remove redundant setting of any in for-loop

2017-11-14 Thread Marciniszyn, Mike 
> > Doug,  do you want a tested patch from me?
> 
> Preferably, yes :-)
> 

These patches are in a two patch series:
https://marc.info/?l=linux-rdma=151069497506923=2

The first is Colin's warning fix and the second is Joe's cleanup.

My value add is the testing.  :)

They pass our psm testing.

Thanks for the patches!

Mike

RE: [PATCH] IB/qib: remove redundant setting of any in for-loop

2017-11-14 Thread Marciniszyn, Mike 
> > Doug,  do you want a tested patch from me?
> 
> Preferably, yes :-)
> 

These patches are in a two patch series:
https://marc.info/?l=linux-rdma=151069497506923=2

The first is Colin's warning fix and the second is Joe's cleanup.

My value add is the testing.  :)

They pass our psm testing.

Thanks for the patches!

Mike

RE: [PATCH] IB/qib: remove redundant setting of any in for-loop

2017-11-10 Thread Marciniszyn, Mike 
> > On Fri, 2017-10-20 at 09:21 +0200, Colin King wrote:
> > > From: Colin Ian King 
> > >
> > > The variable all is being set but is never read after this
> > > hence it can be removed from the for loop initialization.
> > > Cleans up clang warning:
> >
> > any is really used as bool and is initialized at function
> > entry.  The earlier loop also reinitializes any unnecessarily.
> 
> Denny, can you weigh in on what you want in this thread?  Thanks.
> 

I am ok with both Colin's and Joe's patch.

Joe's patch would require additional testing vs. the trivial warning fix.

Then there is a "guideline"  to keep fixes separate from clean up...

Doug,  do you want a tested patch from me?

Mike

RE: [PATCH] IB/qib: remove redundant setting of any in for-loop

2017-11-10 Thread Marciniszyn, Mike 
> > On Fri, 2017-10-20 at 09:21 +0200, Colin King wrote:
> > > From: Colin Ian King 
> > >
> > > The variable all is being set but is never read after this
> > > hence it can be removed from the for loop initialization.
> > > Cleans up clang warning:
> >
> > any is really used as bool and is initialized at function
> > entry.  The earlier loop also reinitializes any unnecessarily.
> 
> Denny, can you weigh in on what you want in this thread?  Thanks.
> 

I am ok with both Colin's and Joe's patch.

Joe's patch would require additional testing vs. the trivial warning fix.

Then there is a "guideline"  to keep fixes separate from clean up...

Doug,  do you want a tested patch from me?

Mike

RE: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied by user verbs cmds

2017-07-17 Thread Marciniszyn, Mike 
> it should be broken, see the patches submitted already to fix it :)

I saw that and responded with Tested-by:

Mike

RE: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied by user verbs cmds

2017-07-17 Thread Marciniszyn, Mike 
> it should be broken, see the patches submitted already to fix it :)

I saw that and responded with Tested-by:

Mike

RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr

2017-07-17 Thread Marciniszyn, Mike 
> Subject: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> 
> Initialize the port_num for iWARP in rdma_init_qp_attr.
> 
> Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds")
> Cc:  # v2.6.14+
> Reviewed-by: Steve Wise 
> Signed-off-by: Mustafa Ismail 

Tested-by: Mike Marciniszyn

RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr

2017-07-17 Thread Marciniszyn, Mike 
> Subject: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> 
> Initialize the port_num for iWARP in rdma_init_qp_attr.
> 
> Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds")
> Cc:  # v2.6.14+
> Reviewed-by: Steve Wise 
> Signed-off-by: Mustafa Ismail 

Tested-by: Mike Marciniszyn

RE: [PATCH v2 1/2] RDMA/uverbs: Fix the check for port number

2017-07-17 Thread Marciniszyn, Mike 
> Subject: [PATCH v2 1/2] RDMA/uverbs: Fix the check for port number
> 
> The port number is only valid if IB_QP_PORT is set in the mask.
> So only check port number if it is valid to prevent modify_qp from
> failing due to an invalid port number.
> 
> Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds")
> Cc:  # v2.6.14+
> Reviewed-by: Steve Wise 
> Signed-off-by: Mustafa Ismail 

Tested-by: Mike Marciniszyn

RE: [PATCH v2 1/2] RDMA/uverbs: Fix the check for port number

2017-07-17 Thread Marciniszyn, Mike 
> Subject: [PATCH v2 1/2] RDMA/uverbs: Fix the check for port number
> 
> The port number is only valid if IB_QP_PORT is set in the mask.
> So only check port number if it is valid to prevent modify_qp from
> failing due to an invalid port number.
> 
> Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds")
> Cc:  # v2.6.14+
> Reviewed-by: Steve Wise 
> Signed-off-by: Mustafa Ismail 

Tested-by: Mike Marciniszyn

RE: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied by user verbs cmds

2017-07-17 Thread Marciniszyn, Mike 
> Subject: Re: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied
> by user verbs cmds
> 
> On Thu, Jul 13, 2017 at 03:54:28PM +, Ismail, Mustafa wrote:
> > > Subject: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied
> by
> > > user verbs cmds
> > >
> > > 4.4-stable review patch.  If anyone has any objections, please let me
> know.
> >
> > Yes, this breaks modify qp.
> > See https://patchwork.kernel.org/patch/9830663/
> 
> I don't understand this response at all, sorry.
> 
> What should I do about this?  Is this patch alone a problem?  Is there
> some other patch I should apply that is in Linus's tree?  Where is the
> problem, only in this old release?
> 
> totally confused,
> 

This patch utterly breaks qib and hfi1 on at least 4.10.11 stable kernel.

I suspect and will soon verify that v4.13-rc1 is broke as well.

Mike

RE: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied by user verbs cmds

2017-07-17 Thread Marciniszyn, Mike 
> Subject: Re: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied
> by user verbs cmds
> 
> On Thu, Jul 13, 2017 at 03:54:28PM +, Ismail, Mustafa wrote:
> > > Subject: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied
> by
> > > user verbs cmds
> > >
> > > 4.4-stable review patch.  If anyone has any objections, please let me
> know.
> >
> > Yes, this breaks modify qp.
> > See https://patchwork.kernel.org/patch/9830663/
> 
> I don't understand this response at all, sorry.
> 
> What should I do about this?  Is this patch alone a problem?  Is there
> some other patch I should apply that is in Linus's tree?  Where is the
> problem, only in this old release?
> 
> totally confused,
> 

This patch utterly breaks qib and hfi1 on at least 4.10.11 stable kernel.

I suspect and will soon verify that v4.13-rc1 is broke as well.

Mike

RE: [PATCH] IB/qib: use rb_entry()

2016-12-20 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/qib: use rb_entry()
> 
> To make the code clearer, use rb_entry() instead of container_of() to deal
> with rbtree.
> 
> Signed-off-by: Geliang Tang 

Thanks for the patch!

Mike

Acked-by: Mike Marciniszyn

RE: [PATCH] IB/qib: use rb_entry()

2016-12-20 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/qib: use rb_entry()
> 
> To make the code clearer, use rb_entry() instead of container_of() to deal
> with rbtree.
> 
> Signed-off-by: Geliang Tang 

Thanks for the patch!

Mike

Acked-by: Mike Marciniszyn

false checkpatch finding?

2016-10-12 Thread Marciniszyn, Mike 
<4.8 tree>/scripts/checkpatch.pl -F foo.h
WARNING: Missing a blank line after declarations
#3: FILE: foo.h:3:
+   unsigned long f1;
+   volatile __le64 f2.

WARNING: Use of volatile is usually wrong: see 
Documentation/volatile-considered-harmful.txt
#3: FILE: foo.h:3:
+   volatile __le64 f2.

total: 0 errors, 2 warnings, 4 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
  mechanically convert to the typical style using --fix or --fix-inplace.

foo.h has style problems, please review.

NOTE: If any of the errors are false positives, please report
  them to the maintainer, see CHECKPATCH in MAINTAINERS.

Adding a gratuitous blank line after f1 silences the bogus warning.

The volatile warning is ok because this is a hardware written field.

Snip the test file from below.

Mike


struct foo {
unsigned long f1;
volatile __le64 f2.
};

false checkpatch finding?

2016-10-12 Thread Marciniszyn, Mike 
<4.8 tree>/scripts/checkpatch.pl -F foo.h
WARNING: Missing a blank line after declarations
#3: FILE: foo.h:3:
+   unsigned long f1;
+   volatile __le64 f2.

WARNING: Use of volatile is usually wrong: see 
Documentation/volatile-considered-harmful.txt
#3: FILE: foo.h:3:
+   volatile __le64 f2.

total: 0 errors, 2 warnings, 4 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
  mechanically convert to the typical style using --fix or --fix-inplace.

foo.h has style problems, please review.

NOTE: If any of the errors are false positives, please report
  them to the maintainer, see CHECKPATCH in MAINTAINERS.

Adding a gratuitous blank line after f1 silences the bogus warning.

The volatile warning is ok because this is a hardware written field.

Snip the test file from below.

Mike


struct foo {
unsigned long f1;
volatile __le64 f2.
};

RE: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation

2016-08-19 Thread Marciniszyn, Mike 
> >
> > The bail_tmp: label is then not needed.
> 
> You still need to free tmp allocation if qib_eeprom_write failed and this is
> your bail_tmp.
> 

Typo.   The bail: label is not needed.

Mike

RE: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation

2016-08-19 Thread Marciniszyn, Mike 
> >
> > The bail_tmp: label is then not needed.
> 
> You still need to free tmp allocation if qib_eeprom_write failed and this is
> your bail_tmp.
> 

Typo.   The bail: label is not needed.

Mike

RE: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation

2016-08-19 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/qib: Use memdup_user() rather than duplicating its
> diff --git a/drivers/infiniband/hw/qib/qib_fs.c

I would be even more aggressive at reducing lines of code.

For example do direct returns when ok to do:
if (pos != 0 || count != sizeof(struct qib_flash))
return -EINVAL;

tmp = memdup_user(buf, count);
if (IS_ERR(tmp))
return PTR_ERR(tmp);

The bail_tmp: label is then not needed.

Mike

RE: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation

2016-08-19 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/qib: Use memdup_user() rather than duplicating its
> diff --git a/drivers/infiniband/hw/qib/qib_fs.c

I would be even more aggressive at reducing lines of code.

For example do direct returns when ok to do:
if (pos != 0 || count != sizeof(struct qib_flash))
return -EINVAL;

tmp = memdup_user(buf, count);
if (IS_ERR(tmp))
return PTR_ERR(tmp);

The bail_tmp: label is then not needed.

Mike

RE: [PATCH 09/22] IB/qib: Remove deprecated create_singlethread_workqueue

2016-08-16 Thread Marciniszyn, Mike 
> Subject: [PATCH 09/22] IB/qib: Remove deprecated
> create_singlethread_workqueue
> 

Looks and tests good!

Tested-by: Mike Marciniszyn 
Acked-by: Mike Marciniszyn

RE: [PATCH 09/22] IB/qib: Remove deprecated create_singlethread_workqueue

2016-08-16 Thread Marciniszyn, Mike 
> Subject: [PATCH 09/22] IB/qib: Remove deprecated
> create_singlethread_workqueue
> 

Looks and tests good!

Tested-by: Mike Marciniszyn 
Acked-by: Mike Marciniszyn

RE: [PATCH RESEND] qib:Fix concurrent access in the function, qib_init_iba6120_funcs

2016-03-07 Thread Marciniszyn, Mike 
> From: nick [mailto:xerofo...@gmail.com]
> Sent: Monday, March 7, 2016 2:21 PM
> To: Marciniszyn, Mike <mike.marcinis...@intel.com>
> Cc: dledf...@redhat.com; Hefty, Sean <sean.he...@intel.com>;
> hal.rosenst...@gmail.com; linux-r...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH RESEND] qib:Fix concurrent access in the function,
> qib_init_iba6120_funcs
> 
> 
> 
> On 2016-03-07 11:34 AM, Marciniszyn, Mike wrote:
> >> This fixes concurrent access in the function, qib_init_iba6120_funcs
> >> by locking around the calls to when setting up f_sendctrl and
> >> f_set_armlauch function pointers to the functions, sendctrl_6120_mod
> >> qib_set_6120_armlaunch due to these functions needing to have their
> >> caller to hold the spinlock that is part of the qib_ibport pointer
> >> they are using and due to the lock not being held by higher up
> >> functions in the caller stack we need to hold it in qib_init_iba6210 to 
> >> avoid
> conncurrent access when setting up these function pointers.
> >>
> >
> > I'm not sure I agree.
> >
> > static struct pci_driver qib_driver = {
> > .name = QIB_DRV_NAME,
> > .probe = qib_init_one,
> > .remove = qib_remove_one,
> > .id_table = qib_pci_tbl,
> > .err_handler = _pci_err_handler, };
> >
> > The only caller is the probe function, which naturally protects since the 
> > device
> cannot be used until the probe returns.
> >
> That's correct my concern was about threads from other kernel contexts but
> then again that's probably me being too conservative about critical region
> protection.
> Cheers,
> Nick

Doug, I'm NAKing this patch based on this reply from Nick.

Given that function vectors cannot be used until the qib_init_iba6120_funcs() 
functions return, this patch just not needed.

Mike

RE: [PATCH RESEND] qib:Fix concurrent access in the function, qib_init_iba6120_funcs

2016-03-07 Thread Marciniszyn, Mike 
> From: nick [mailto:xerofo...@gmail.com]
> Sent: Monday, March 7, 2016 2:21 PM
> To: Marciniszyn, Mike 
> Cc: dledf...@redhat.com; Hefty, Sean ;
> hal.rosenst...@gmail.com; linux-r...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH RESEND] qib:Fix concurrent access in the function,
> qib_init_iba6120_funcs
> 
> 
> 
> On 2016-03-07 11:34 AM, Marciniszyn, Mike wrote:
> >> This fixes concurrent access in the function, qib_init_iba6120_funcs
> >> by locking around the calls to when setting up f_sendctrl and
> >> f_set_armlauch function pointers to the functions, sendctrl_6120_mod
> >> qib_set_6120_armlaunch due to these functions needing to have their
> >> caller to hold the spinlock that is part of the qib_ibport pointer
> >> they are using and due to the lock not being held by higher up
> >> functions in the caller stack we need to hold it in qib_init_iba6210 to 
> >> avoid
> conncurrent access when setting up these function pointers.
> >>
> >
> > I'm not sure I agree.
> >
> > static struct pci_driver qib_driver = {
> > .name = QIB_DRV_NAME,
> > .probe = qib_init_one,
> > .remove = qib_remove_one,
> > .id_table = qib_pci_tbl,
> > .err_handler = _pci_err_handler, };
> >
> > The only caller is the probe function, which naturally protects since the 
> > device
> cannot be used until the probe returns.
> >
> That's correct my concern was about threads from other kernel contexts but
> then again that's probably me being too conservative about critical region
> protection.
> Cheers,
> Nick

Doug, I'm NAKing this patch based on this reply from Nick.

Given that function vectors cannot be used until the qib_init_iba6120_funcs() 
functions return, this patch just not needed.

Mike

RE: [PATCH RESEND] qib:Fix concurrent access in the function, qib_init_iba6120_funcs

2016-03-07 Thread Marciniszyn, Mike 
> This fixes concurrent access in the function, qib_init_iba6120_funcs by 
> locking
> around the calls to when setting up f_sendctrl and f_set_armlauch function
> pointers to the functions, sendctrl_6120_mod qib_set_6120_armlaunch due to
> these functions needing to have their caller to hold the spinlock that is 
> part of
> the qib_ibport pointer they are using and due to the lock not being held by
> higher up functions in the caller stack we need to hold it in 
> qib_init_iba6210 to
> avoid conncurrent access when setting up these function pointers.
> 

I'm not sure I agree.

static struct pci_driver qib_driver = {
.name = QIB_DRV_NAME,
.probe = qib_init_one,
.remove = qib_remove_one,
.id_table = qib_pci_tbl,
.err_handler = _pci_err_handler,
};

The only caller is the probe function, which naturally protects since the 
device cannot be used until the probe returns.

Are you actually having an issue with this older version of a qib card?

The other board specific routines if this indeed is an issue, would need to be 
fixed as well since that have the same chip specific routine.

Mike

RE: [PATCH RESEND] qib:Fix concurrent access in the function, qib_init_iba6120_funcs

2016-03-07 Thread Marciniszyn, Mike 
> This fixes concurrent access in the function, qib_init_iba6120_funcs by 
> locking
> around the calls to when setting up f_sendctrl and f_set_armlauch function
> pointers to the functions, sendctrl_6120_mod qib_set_6120_armlaunch due to
> these functions needing to have their caller to hold the spinlock that is 
> part of
> the qib_ibport pointer they are using and due to the lock not being held by
> higher up functions in the caller stack we need to hold it in 
> qib_init_iba6210 to
> avoid conncurrent access when setting up these function pointers.
> 

I'm not sure I agree.

static struct pci_driver qib_driver = {
.name = QIB_DRV_NAME,
.probe = qib_init_one,
.remove = qib_remove_one,
.id_table = qib_pci_tbl,
.err_handler = _pci_err_handler,
};

The only caller is the probe function, which naturally protects since the 
device cannot be used until the probe returns.

Are you actually having an issue with this older version of a qib card?

The other board specific routines if this indeed is an issue, would need to be 
fixed as well since that have the same chip specific routine.

Mike

RE: [PATCH] staging: rdma: hfi1: diag: constify hfi1_filter_array structure

2016-01-04 Thread Marciniszyn, Mike 
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Julia Lawall
> Subject: [PATCH] staging: rdma: hfi1: diag: constify hfi1_filter_array
> structure
> 
> The hfi1_filter_array structure is never modified, so declare it as const.
> 
> Done with the help of Coccinelle.
> 
> Signed-off-by: Julia Lawall 
> 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] staging: rdma: hfi1: diag: constify hfi1_filter_array structure

2016-01-04 Thread Marciniszyn, Mike 
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Julia Lawall
> Subject: [PATCH] staging: rdma: hfi1: diag: constify hfi1_filter_array
> structure
> 
> The hfi1_filter_array structure is never modified, so declare it as const.
> 
> Done with the help of Coccinelle.
> 
> Signed-off-by: Julia Lawall 
> 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/3] staging/rdma/hfi1: consolidate kmalloc_array+memset into kcalloc

2015-12-14 Thread Marciniszyn, Mike 
> --- a/drivers/staging/rdma/hfi1/chip.c
> +++ b/drivers/staging/rdma/hfi1/chip.c
> @@ -10128,8 +10128,7 @@ static void init_qos(struct hfi1_devdata *dd,
> u32 first_ctxt)
>   goto bail;
>   if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
>   goto bail;
> - rsmmap = kmalloc_array(NUM_MAP_REGS, sizeof(u64),
> GFP_KERNEL);
> - memset(rsmmap, rxcontext, NUM_MAP_REGS * sizeof(u64));
> + rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
>   /* init the local copy of the table */
>   for (i = 0, ctxt = first_ctxt; i < num_vls; i++) {
>   unsigned tctxt;
> --

I'm NAKing this.

There is a chip specific difference that accounts for the current code.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] staging/rdma/hfi1: check return value of kcalloc

2015-12-14 Thread Marciniszyn, Mike 
> @@ -10129,6 +10129,9 @@ static void init_qos(struct hfi1_devdata *dd,
> u32 first_ctxt)
>   if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
>   goto bail;
>   rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
> + if (!rsmmap)
> + goto bail;
> +

I checked out a linux-next remote at the next-20151214 tag.

The allocation method is clearly kmalloc_array() not kcalloc().

Where are you seeing the kcalloc()?

While it is tempting to allocate and zero, there is a chip rev specific 
difference.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/3] staging/rdma/hfi1: check return value of kcalloc

2015-12-14 Thread Marciniszyn, Mike 
> @@ -10129,6 +10129,9 @@ static void init_qos(struct hfi1_devdata *dd,
> u32 first_ctxt)
>   if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
>   goto bail;
>   rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
> + if (!rsmmap)
> + goto bail;
> +

I checked out a linux-next remote at the next-20151214 tag.

The allocation method is clearly kmalloc_array() not kcalloc().

Where are you seeing the kcalloc()?

While it is tempting to allocate and zero, there is a chip rev specific 
difference.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/3] staging/rdma/hfi1: consolidate kmalloc_array+memset into kcalloc

2015-12-14 Thread Marciniszyn, Mike 
> --- a/drivers/staging/rdma/hfi1/chip.c
> +++ b/drivers/staging/rdma/hfi1/chip.c
> @@ -10128,8 +10128,7 @@ static void init_qos(struct hfi1_devdata *dd,
> u32 first_ctxt)
>   goto bail;
>   if (num_vls * qpns_per_vl > dd->chip_rcv_contexts)
>   goto bail;
> - rsmmap = kmalloc_array(NUM_MAP_REGS, sizeof(u64),
> GFP_KERNEL);
> - memset(rsmmap, rxcontext, NUM_MAP_REGS * sizeof(u64));
> + rsmmap = kcalloc(NUM_MAP_REGS, sizeof(u64), GFP_KERNEL);
>   /* init the local copy of the table */
>   for (i = 0, ctxt = first_ctxt; i < num_vls; i++) {
>   unsigned tctxt;
> --

I'm NAKing this.

There is a chip specific difference that accounts for the current code.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2 1/1] staging: rdma: hfi1 : Prefer using the BIT macro

2015-11-06 Thread Marciniszyn, Mike 
> Cc: Sunny Kumar
> Subject: [PATCH v2 1/1] staging: rdma: hfi1 : Prefer using the BIT macro
> 
> This patch replaces bit shifting on 1 with the BIT(x) macro
> 
> Signed-off-by: Sunny Kumar 
> ---

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2 1/1] staging: rdma: hfi1 : Prefer using the BIT macro

2015-11-06 Thread Marciniszyn, Mike 
> Cc: Sunny Kumar
> Subject: [PATCH v2 1/1] staging: rdma: hfi1 : Prefer using the BIT macro
> 
> This patch replaces bit shifting on 1 with the BIT(x) macro
> 
> Signed-off-by: Sunny Kumar 
> ---

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] staging: rdma: hfi1 : Prefer using the BIT macro

2015-11-05 Thread Marciniszyn, Mike 
> Subject: [PATCH 1/1] staging: rdma: hfi1 : Prefer using the BIT macro
> 
> This patch replaces bit shifting on 1 with the BIT(x) macro
> 
> Signed-off-by: Sunny Kumar 
> ---

Nak.

The patch leaves the shift in.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/1] staging: rdma: hfi1 : Prefer using the BIT macro

2015-11-05 Thread Marciniszyn, Mike 
> Subject: [PATCH 1/1] staging: rdma: hfi1 : Prefer using the BIT macro
> 
> This patch replaces bit shifting on 1 with the BIT(x) macro
> 
> Signed-off-by: Sunny Kumar 
> ---

Nak.

The patch leaves the shift in.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] fix return value error

2015-10-14 Thread Marciniszyn, Mike 
> > Subject: [PATCH] fix return value error
> >
> 
> I checked returns in configfs (-ENOMEM), proc (-ENOENT), proc-sys (-
> ENOMEM), ramfs (-ENOSPC), vfs (-ENOMEM).
> 
> Not entirely consistent but this matches the majority.
> 
> I agree -EPERM is pretty misleading.
> 
> Acked-by: Mike Marciniszyn 

Thanks Or on catching the bad directory in the patch.

Doug, can you fix this up or do you want it resubmitted?

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] fix return value error

2015-10-14 Thread Marciniszyn, Mike 
> Subject: [PATCH] fix return value error
> 

I checked returns in configfs (-ENOMEM), proc (-ENOENT), proc-sys (-ENOMEM), 
ramfs (-ENOSPC), vfs (-ENOMEM).

Not entirely consistent but this matches the majority.

I agree -EPERM is pretty misleading.

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] fix return value error

2015-10-14 Thread Marciniszyn, Mike 
> > Subject: [PATCH] fix return value error
> >
> 
> I checked returns in configfs (-ENOMEM), proc (-ENOENT), proc-sys (-
> ENOMEM), ramfs (-ENOSPC), vfs (-ENOMEM).
> 
> Not entirely consistent but this matches the majority.
> 
> I agree -EPERM is pretty misleading.
> 
> Acked-by: Mike Marciniszyn 

Thanks Or on catching the bad directory in the patch.

Doug, can you fix this up or do you want it resubmitted?

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] fix return value error

2015-10-14 Thread Marciniszyn, Mike 
> Subject: [PATCH] fix return value error
> 

I checked returns in configfs (-ENOMEM), proc (-ENOENT), proc-sys (-ENOMEM), 
ramfs (-ENOSPC), vfs (-ENOMEM).

Not entirely consistent but this matches the majority.

I agree -EPERM is pretty misleading.

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] staging/rdma/hfi1: do not use u8 to store a 32-bit integer

2015-09-21 Thread Marciniszyn, Mike 
> Subject: [PATCH] staging/rdma/hfi1: do not use u8 to store a 32-bit integer
> 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] IB/hfi1: class_name_user() should be static

2015-09-21 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/hfi1: class_name_user() should be static
> 
> Fixes the following sparse warning:
>   drivers/staging/rdma/hfi1/device.c:127:12:
>   warning: symbol 'class_name_user' was not declared. Should it be static?
> 
> Signed-off-by: Geliang Tang 
> ---

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] IB/hfi1: use kvfree() in sdma.c

2015-09-21 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/hfi1: use kvfree() in sdma.c
> 
> Use kvfree() instead of open-coding it.
> 
> Signed-off-by: Geliang Tang 
> ---
>  drivers/staging/rdma/hfi1/sdma.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)

Thanks for the patch.

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] IB/hfi1: use kvfree() in sdma.c

2015-09-21 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/hfi1: use kvfree() in sdma.c
> 
> Use kvfree() instead of open-coding it.
> 
> Signed-off-by: Geliang Tang 
> ---
>  drivers/staging/rdma/hfi1/sdma.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)

Thanks for the patch.

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] IB/hfi1: class_name_user() should be static

2015-09-21 Thread Marciniszyn, Mike 
> Subject: [PATCH] IB/hfi1: class_name_user() should be static
> 
> Fixes the following sparse warning:
>   drivers/staging/rdma/hfi1/device.c:127:12:
>   warning: symbol 'class_name_user' was not declared. Should it be static?
> 
> Signed-off-by: Geliang Tang 
> ---

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] staging/rdma/hfi1: do not use u8 to store a 32-bit integer

2015-09-21 Thread Marciniszyn, Mike 
> Subject: [PATCH] staging/rdma/hfi1: do not use u8 to store a 32-bit integer
> 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 11/39] hfi1: drop null test before destroy functions

2015-09-15 Thread Marciniszyn, Mike 
> Subject: [PATCH 11/39] hfi1: drop null test before destroy functions
> 
> Remove unneeded NULL test.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // 
> @@ expression x; @@
> -if (x != NULL)
>   \(kmem_cache_destroy\|mempool_destroy\|dma_pool_destroy\)(x);
> // 
> 
> Signed-off-by: Julia Lawall 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 11/39] hfi1: drop null test before destroy functions

2015-09-15 Thread Marciniszyn, Mike 
> Subject: [PATCH 11/39] hfi1: drop null test before destroy functions
> 
> Remove unneeded NULL test.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // 
> @@ expression x; @@
> -if (x != NULL)
>   \(kmem_cache_destroy\|mempool_destroy\|dma_pool_destroy\)(x);
> // 
> 
> Signed-off-by: Julia Lawall 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix

2015-08-18 Thread Marciniszyn, Mike 
> Subject: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix
> 
> Remove the 'CONFIG_' prefix of the Kconfig options PRESCAN_RXQ and
> SDMA_VERBOSITY in Kconfig.  Such prefix in Kconfig requires a double
> 'CONFIG_CONFIG_' prefix in Make and CPP syntax.
> 

Doug,  can you pull this one in?

There is a Kconfig checker that caught this one!

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: linux-next: Tree for Aug 17 (staging/hfi1)

2015-08-18 Thread Marciniszyn, Mike 
> A:  these and other similar errors:
> 
> In file included from ../drivers/staging/hfi1/chip.c:61:0:
> ../drivers/staging/hfi1/chip.c: In function ‘__hfi1_trace_LINKVERB’:
> ../drivers/staging/hfi1/trace.h:1357:20: error: function
> ‘__hfi1_trace_LINKVERB’ can never be inlined because it uses variable
> argument lists  static inline void __hfi1_trace_##fn(const char *func, char
> *fmt, ...) \
>

This was corrected with
https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg27221.html, which I 
think Doug has picked up.

Doug, Randy, Stephen how do we want to handle fixing drivers/staging/hfi1 via 
the staging list vs. the linux-rdma list?

> B:  When CONFIG_PCI is not enabled, lots of these errors:
> 

The driver used to get a gift from the drivers/infiniband/hw/Kconfig, which has 
the required PCI dependency.

The infiniband dependency is also similarly missing and was resolved via the 
following temporary patch:

https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg26989.html

This one also needs to be similarly added for that 0-day build issue.

I have submitted to both lists 
https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg27299.html assuming 
the above one as a dependency.

> In file included from ../drivers/staging/hfi1/chip.c:60:0:
> ../drivers/staging/hfi1/hfi.h:508:20: error: field ‘msix’ has incomplete
> type
>   struct msix_entry msix;
>^
> 
> ../drivers/staging/hfi1/chip.c: In function ‘disable_intx’:
> ../drivers/staging/hfi1/chip.c:8667:2: error: implicit declaration of function
> ‘pci_intx’ [-Werror=implicit-function-declaration]
>   pci_intx(pdev, 0);
>   ^
> 
> so the driver should depend on PCI or maybe even PCI_MSI.
> 
> C.  Please provide some contact info, e.g. in a TODO file or the MAINTAINERS
> file.
> 

I have submitted 
https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg27298.html to both 
lists to add a maintainer.

Mike

RE: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix

2015-08-18 Thread Marciniszyn, Mike 
 Subject: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix
 
 Remove the 'CONFIG_' prefix of the Kconfig options PRESCAN_RXQ and
 SDMA_VERBOSITY in Kconfig.  Such prefix in Kconfig requires a double
 'CONFIG_CONFIG_' prefix in Make and CPP syntax.
 

Doug,  can you pull this one in?

There is a Kconfig checker that caught this one!

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: linux-next: Tree for Aug 17 (staging/hfi1)

2015-08-18 Thread Marciniszyn, Mike 
 A:  these and other similar errors:
 
 In file included from ../drivers/staging/hfi1/chip.c:61:0:
 ../drivers/staging/hfi1/chip.c: In function ‘__hfi1_trace_LINKVERB’:
 ../drivers/staging/hfi1/trace.h:1357:20: error: function
 ‘__hfi1_trace_LINKVERB’ can never be inlined because it uses variable
 argument lists  static inline void __hfi1_trace_##fn(const char *func, char
 *fmt, ...) \


This was corrected with
https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg27221.html, which I 
think Doug has picked up.

Doug, Randy, Stephen how do we want to handle fixing drivers/staging/hfi1 via 
the staging list vs. the linux-rdma list?

 B:  When CONFIG_PCI is not enabled, lots of these errors:
 

The driver used to get a gift from the drivers/infiniband/hw/Kconfig, which has 
the required PCI dependency.

The infiniband dependency is also similarly missing and was resolved via the 
following temporary patch:

https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg26989.html

This one also needs to be similarly added for that 0-day build issue.

I have submitted to both lists 
https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg27299.html assuming 
the above one as a dependency.

 In file included from ../drivers/staging/hfi1/chip.c:60:0:
 ../drivers/staging/hfi1/hfi.h:508:20: error: field ‘msix’ has incomplete
 type
   struct msix_entry msix;
^
 
 ../drivers/staging/hfi1/chip.c: In function ‘disable_intx’:
 ../drivers/staging/hfi1/chip.c:8667:2: error: implicit declaration of function
 ‘pci_intx’ [-Werror=implicit-function-declaration]
   pci_intx(pdev, 0);
   ^
 
 so the driver should depend on PCI or maybe even PCI_MSI.
 
 C.  Please provide some contact info, e.g. in a TODO file or the MAINTAINERS
 file.
 

I have submitted 
https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg27298.html to both 
lists to add a maintainer.

Mike

RE: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix

2015-08-17 Thread Marciniszyn, Mike 
> Subject: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix
> Signed-off-by: Valentin Rothberg 
> ---

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix

2015-08-17 Thread Marciniszyn, Mike 
 Subject: [PATCH] staging: hfi1: Kconfig: remove 'CONFIG_' prefix
 Signed-off-by: Valentin Rothberg valentinrothb...@gmail.com
 ---

Thanks for the patch!

Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v3 2/3] IB/qib: use arch_phys_wc_add()

2015-04-21 Thread Marciniszyn, Mike 
> > This driver already makes use of ioremap_wc() on PIO buffers, so
> > convert it to use arch_phys_wc_add().
> 
> This is probably OK, but I think you should also remove the qib_wc_pat module
> parameter.
> 
> Jason

Revise based on Jason's request and I will do some testing.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v3 2/3] IB/qib: use arch_phys_wc_add()

2015-04-21 Thread Marciniszyn, Mike 
  This driver already makes use of ioremap_wc() on PIO buffers, so
  convert it to use arch_phys_wc_add().
 
 This is probably OK, but I think you should also remove the qib_wc_pat module
 parameter.
 
 Jason

Revise based on Jason's request and I will do some testing.

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] infiniband: hw: ipath: ipath_wc_ppc64.c: Remove unused function

2015-01-16 Thread Marciniszyn, Mike 
> From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> ow...@vger.kernel.org] On Behalf Of Rickard Strandqvist
> Sent: Saturday, December 20, 2014 11:19 AM
> To: infinipath; Roland Dreier
> Cc: Rickard Strandqvist; Hefty, Sean; Hal Rosenstock; linux-
> r...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] infiniband: hw: ipath: ipath_wc_ppc64.c: Remove unused
> function
> 

Roland,

Can you change in summary from https://patchwork.kernel.org/patch/5522791:

IB/ipath: Remove unused function in ipath_wc_ppc64

Thanks for the patch!
Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] IB: qib: qib_iba7322: Remove unused function

2015-01-16 Thread Marciniszyn, Mike 
> From: Rickard Strandqvist [mailto:rickard_strandqv...@spectrumdigital.se]
> Sent: Sunday, January 11, 2015 9:05 AM
> Subject: [PATCH] IB: qib: qib_iba7322: Remove unused function
> 
Roland, can you change the summary to:
IB/qib: Remove unused function in qib_iba7322

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] IB: qib: qib_iba7322: Remove unused function

2015-01-16 Thread Marciniszyn, Mike 
 From: Rickard Strandqvist [mailto:rickard_strandqv...@spectrumdigital.se]
 Sent: Sunday, January 11, 2015 9:05 AM
 Subject: [PATCH] IB: qib: qib_iba7322: Remove unused function
 
Roland, can you change the summary to:
IB/qib: Remove unused function in qib_iba7322

Thanks for the patch!

Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] infiniband: hw: ipath: ipath_wc_ppc64.c: Remove unused function

2015-01-16 Thread Marciniszyn, Mike 
 From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
 ow...@vger.kernel.org] On Behalf Of Rickard Strandqvist
 Sent: Saturday, December 20, 2014 11:19 AM
 To: infinipath; Roland Dreier
 Cc: Rickard Strandqvist; Hefty, Sean; Hal Rosenstock; linux-
 r...@vger.kernel.org; linux-kernel@vger.kernel.org
 Subject: [PATCH] infiniband: hw: ipath: ipath_wc_ppc64.c: Remove unused
 function
 

Roland,

Can you change in summary from https://patchwork.kernel.org/patch/5522791:

IB/ipath: Remove unused function in ipath_wc_ppc64

Thanks for the patch!
Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] qib: qib_qp: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

2014-09-19 Thread Marciniszyn, Mike 
> Subject: [PATCH] qib: qib_qp: Replace rcu_assign_pointer() with
> RCU_INIT_POINTER()
>

Why not consolidate this with 
http://marc.info/?l=linux-rdma=140836578119485=2 so there is just one patch?

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] qib_keys: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

2014-09-19 Thread Marciniszyn, Mike 
> Subject: [PATCH] qib_keys: Replace rcu_assign_pointer() with
> RCU_INIT_POINTER()
> 

I would prefer the summary be:
IB/qib: qib_remove_lkey()  Replace rcu_assign_pointer() with
> RCU_INIT_POINTER()

Otherwise the patch looks ok and has been tested.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] qib_qp: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

2014-09-19 Thread Marciniszyn, Mike 
> Subject: [PATCH] qib_qp: Replace rcu_assign_pointer() with
> RCU_INIT_POINTER()
> 

I would prefer the summary line be:
IB/qib: remove_qp() use RCU_INIT_POINTER() instead of rcu_assign_pointer()

Otherwise I agree with the patch and I have tested it with large numbers of QPs.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] qib_qp: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

2014-09-19 Thread Marciniszyn, Mike 
 Subject: [PATCH] qib_qp: Replace rcu_assign_pointer() with
 RCU_INIT_POINTER()
 

I would prefer the summary line be:
IB/qib: remove_qp() use RCU_INIT_POINTER() instead of rcu_assign_pointer()

Otherwise I agree with the patch and I have tested it with large numbers of QPs.

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] qib_keys: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

2014-09-19 Thread Marciniszyn, Mike 
 Subject: [PATCH] qib_keys: Replace rcu_assign_pointer() with
 RCU_INIT_POINTER()
 

I would prefer the summary be:
IB/qib: qib_remove_lkey()  Replace rcu_assign_pointer() with
 RCU_INIT_POINTER()

Otherwise the patch looks ok and has been tested.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] qib: qib_qp: Replace rcu_assign_pointer() with RCU_INIT_POINTER()

2014-09-19 Thread Marciniszyn, Mike 
 Subject: [PATCH] qib: qib_qp: Replace rcu_assign_pointer() with
 RCU_INIT_POINTER()


Why not consolidate this with 
http://marc.info/?l=linux-rdmam=140836578119485w=2 so there is just one patch?

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 9/10] IB/qib: use safer test on the result of find_first_zero_bit

2014-06-04 Thread Marciniszyn, Mike 
> 
> No, it's not necessary.  It turns out that the result cannot be greater than 
> the
> requested maximum value.
> 
> Julia

Ok. 

No stable then.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 9/10] IB/qib: use safer test on the result of find_first_zero_bit

2014-06-04 Thread Marciniszyn, Mike 
> Subject: [PATCH 9/10] IB/qib: use safer test on the result of 
> find_first_zero_bit
> 
> From: Julia Lawall 

Thanks for the patch!

Roland, I'm marking this as stable since a memory corruption can occur in the 
_set_bit().

Cc: 
Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 9/10] IB/qib: use safer test on the result of find_first_zero_bit

2014-06-04 Thread Marciniszyn, Mike 
 Subject: [PATCH 9/10] IB/qib: use safer test on the result of 
 find_first_zero_bit
 
 From: Julia Lawall julia.law...@lip6.fr

Thanks for the patch!

Roland, I'm marking this as stable since a memory corruption can occur in the 
_set_bit().

Cc: sta...@vger.kernel.org
Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 9/10] IB/qib: use safer test on the result of find_first_zero_bit

2014-06-04 Thread Marciniszyn, Mike 
 
 No, it's not necessary.  It turns out that the result cannot be greater than 
 the
 requested maximum value.
 
 Julia

Ok. 

No stable then.

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in hw/qib/qib_keys.c

2014-03-31 Thread Marciniszyn, Mike 
> Subject: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in
> hw/qib/qib_keys.c
> 
> Signed-off-by: Monam Agarwal 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in hw/qib/qib_qp.c

2014-03-31 Thread Marciniszyn, Mike 
> Subject: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in
> hw/qib/qib_qp.c
> 
> Signed-off-by: Monam Agarwal 

Thanks for the patch!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in hw/qib/qib_qp.c

2014-03-31 Thread Marciniszyn, Mike 
 Subject: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in
 hw/qib/qib_qp.c
 
 Signed-off-by: Monam Agarwal monamagarwal...@gmail.com

Thanks for the patch!

Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in hw/qib/qib_keys.c

2014-03-31 Thread Marciniszyn, Mike 
 Subject: [PATCH] drivers/infiniband: Use RCU_INIT_POINTER(x, NULL) in
 hw/qib/qib_keys.c
 
 Signed-off-by: Monam Agarwal monamagarwal...@gmail.com

Thanks for the patch!

Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH infiniband] IB/qib: qib_user_sdma_rb_root can be static

2014-03-20 Thread Marciniszyn, Mike 
> Subject: [PATCH infiniband] IB/qib: qib_user_sdma_rb_root can be static
> 
> CC: CQ Tang 
> CC: Roland Dreier 
> Signed-off-by: Fengguang Wu 

Thanks for the patch.

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH infiniband] IB/qib: qib_user_sdma_rb_root can be static

2014-03-20 Thread Marciniszyn, Mike 
 Subject: [PATCH infiniband] IB/qib: qib_user_sdma_rb_root can be static
 
 CC: CQ Tang cq.t...@intel.com
 CC: Roland Dreier rol...@purestorage.com
 Signed-off-by: Fengguang Wu fengguang...@intel.com

Thanks for the patch.

Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of pci_enable_msix()

2014-02-19 Thread Marciniszyn, Mike 
> Subject: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of
> pci_enable_msix()
> 
> As result of deprecation of MSI-X/MSI enablement functions
> pci_enable_msix() and pci_enable_msi_block() all drivers using these two
> interfaces need to be updated to use the new pci_enable_msi_range() and
> pci_enable_msix_range() interfaces.
> 
> Signed-off-by: Alexander Gordeev 

This worked fine!

Roland, can you fix up the subject to change qib to IB/qib when you merge it?

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of pci_enable_msix()

2014-02-19 Thread Marciniszyn, Mike 
 Subject: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of
 pci_enable_msix()
 
 As result of deprecation of MSI-X/MSI enablement functions
 pci_enable_msix() and pci_enable_msi_block() all drivers using these two
 interfaces need to be updated to use the new pci_enable_msi_range() and
 pci_enable_msix_range() interfaces.
 
 Signed-off-by: Alexander Gordeev agord...@redhat.com

This worked fine!

Roland, can you fix up the subject to change qib to IB/qib when you merge it?

Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of pci_enable_msix()

2014-02-18 Thread Marciniszyn, Mike 
> Subject: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of
> pci_enable_msix()
> 
We are testing this now.

I suggest a subject/summary adding the IB/:
IB/qib: Use pci_enable_msix_range() instead of pci_enable_msix()

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of pci_enable_msix()

2014-02-18 Thread Marciniszyn, Mike 
 Subject: [PATCH 2/2] qib: Use pci_enable_msix_range() instead of
 pci_enable_msix()
 
We are testing this now.

I suggest a subject/summary adding the IB/:
IB/qib: Use pci_enable_msix_range() instead of pci_enable_msix()

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: qib_lookup_qpn() appears to leak pointer out of rcu_read_unlock()

2014-02-12 Thread Marciniszyn, Mike 
BTW, I am considering eliminating the atomic_inc() in favor of widening the 
scope of the rcu lock expanse.

Mike

> -Original Message-
> From: Paul E. McKenney [mailto:paul...@linux.vnet.ibm.com]
> Sent: Wednesday, February 12, 2014 9:56 AM
> To: Marciniszyn, Mike
> Cc: rol...@kernel.org; Hefty, Sean; hal.rosenst...@gmail.com; linux-
> r...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: qib_lookup_qpn() appears to leak pointer out of rcu_read_unlock()
> 
> On Wed, Feb 12, 2014 at 01:59:30PM +, Marciniszyn, Mike wrote:
> > > So what am I missing here?
> > >
> >
> > The atomic increment of a reference count:
> 
> Got it, thank you, apologies for the noise!
> 
>   Thanx, Paul
> 
> > struct qib_qp *qib_lookup_qpn(struct qib_ibport *ibp, u32 qpn) {
> > struct qib_qp *qp = NULL;
> >
> > rcu_read_lock();
> > if (unlikely(qpn <= 1)) {
> > if (qpn == 0)
> > qp = rcu_dereference(ibp->qp0);
> > else
> > qp = rcu_dereference(ibp->qp1);
> > if (qp)
> > atomic_inc(>refcount); 
> > <--
> > } else {
> > struct qib_ibdev *dev = _from_ibp(ibp)->dd->verbs_dev;
> > unsigned n = qpn_hash(dev, qpn);
> >
> > for (qp = rcu_dereference(dev->qp_table[n]); qp;
> > qp = rcu_dereference(qp->next))
> > if (qp->ibqp.qp_num == qpn) {
> > atomic_inc(>refcount); 
> > <-
> > break;
> > }
> > }
> > rcu_read_unlock();
> > return qp;
> > }
> >
> > Mike
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: qib_lookup_qpn() appears to leak pointer out of rcu_read_unlock()

2014-02-12 Thread Marciniszyn, Mike 
> So what am I missing here?
> 

The atomic increment of a reference count:

struct qib_qp *qib_lookup_qpn(struct qib_ibport *ibp, u32 qpn)
{
struct qib_qp *qp = NULL;

rcu_read_lock();
if (unlikely(qpn <= 1)) {
if (qpn == 0)
qp = rcu_dereference(ibp->qp0);
else
qp = rcu_dereference(ibp->qp1);
if (qp)
atomic_inc(>refcount); <--
} else {
struct qib_ibdev *dev = _from_ibp(ibp)->dd->verbs_dev;
unsigned n = qpn_hash(dev, qpn);

for (qp = rcu_dereference(dev->qp_table[n]); qp;
qp = rcu_dereference(qp->next))
if (qp->ibqp.qp_num == qpn) {
atomic_inc(>refcount); 
<-
break;
}
}
rcu_read_unlock();
return qp;
}

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: qib_lookup_qpn() appears to leak pointer out of rcu_read_unlock()

2014-02-12 Thread Marciniszyn, Mike 
 So what am I missing here?
 

The atomic increment of a reference count:

struct qib_qp *qib_lookup_qpn(struct qib_ibport *ibp, u32 qpn)
{
struct qib_qp *qp = NULL;

rcu_read_lock();
if (unlikely(qpn = 1)) {
if (qpn == 0)
qp = rcu_dereference(ibp-qp0);
else
qp = rcu_dereference(ibp-qp1);
if (qp)
atomic_inc(qp-refcount); --
} else {
struct qib_ibdev *dev = ppd_from_ibp(ibp)-dd-verbs_dev;
unsigned n = qpn_hash(dev, qpn);

for (qp = rcu_dereference(dev-qp_table[n]); qp;
qp = rcu_dereference(qp-next))
if (qp-ibqp.qp_num == qpn) {
atomic_inc(qp-refcount); 
-
break;
}
}
rcu_read_unlock();
return qp;
}

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: qib_lookup_qpn() appears to leak pointer out of rcu_read_unlock()

2014-02-12 Thread Marciniszyn, Mike 
BTW, I am considering eliminating the atomic_inc() in favor of widening the 
scope of the rcu lock expanse.

Mike

 -Original Message-
 From: Paul E. McKenney [mailto:paul...@linux.vnet.ibm.com]
 Sent: Wednesday, February 12, 2014 9:56 AM
 To: Marciniszyn, Mike
 Cc: rol...@kernel.org; Hefty, Sean; hal.rosenst...@gmail.com; linux-
 r...@vger.kernel.org; linux-kernel@vger.kernel.org
 Subject: Re: qib_lookup_qpn() appears to leak pointer out of rcu_read_unlock()
 
 On Wed, Feb 12, 2014 at 01:59:30PM +, Marciniszyn, Mike wrote:
   So what am I missing here?
  
 
  The atomic increment of a reference count:
 
 Got it, thank you, apologies for the noise!
 
   Thanx, Paul
 
  struct qib_qp *qib_lookup_qpn(struct qib_ibport *ibp, u32 qpn) {
  struct qib_qp *qp = NULL;
 
  rcu_read_lock();
  if (unlikely(qpn = 1)) {
  if (qpn == 0)
  qp = rcu_dereference(ibp-qp0);
  else
  qp = rcu_dereference(ibp-qp1);
  if (qp)
  atomic_inc(qp-refcount); 
  --
  } else {
  struct qib_ibdev *dev = ppd_from_ibp(ibp)-dd-verbs_dev;
  unsigned n = qpn_hash(dev, qpn);
 
  for (qp = rcu_dereference(dev-qp_table[n]); qp;
  qp = rcu_dereference(qp-next))
  if (qp-ibqp.qp_num == qpn) {
  atomic_inc(qp-refcount); 
  -
  break;
  }
  }
  rcu_read_unlock();
  return qp;
  }
 
  Mike
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: linux-next: build warning after merge of the infiniband tree

2013-11-04 Thread Marciniszyn, Mike 
This issue was caught by Tetsuo Handa and Acked on 10/30: 
http://marc.info/?t=13831336458=1=2.

Roland, I noticed that the Tetsuo's original message didn't cc the linux-rdma 
list?

Mike

> -Original Message-
> From: Stephen Rothwell [mailto:s...@canb.auug.org.au]
> Sent: Sunday, November 03, 2013 11:55 PM
> To: Roland Dreier; linux-r...@vger.kernel.org
> Cc: linux-n...@vger.kernel.org; linux-kernel@vger.kernel.org; Jan Kara;
> Marciniszyn, Mike
> Subject: linux-next: build warning after merge of the infiniband tree
> 
> Hi all,
> 
> After merging the infiniband tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
> 
> drivers/infiniband/hw/ipath/ipath_user_sdma.c: In function
> 'ipath_user_sdma_pin_pages':
> drivers/infiniband/hw/ipath/ipath_user_sdma.c:283:6: warning: 'j' is used
> uninitialized in this function [-Wuninitialized]
>   ret = get_user_pages_fast(addr, j, 0, pages);
>   ^
> 
> Introduced by commit 18fec3c6bdcb ("IB/ipath: Convert
> ipath_user_sdma_pin_pages() to use get_user_pages_fast()").  How did that
> pass review or testing?
> 
> --
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: linux-next: build warning after merge of the infiniband tree

2013-11-04 Thread Marciniszyn, Mike 
This issue was caught by Tetsuo Handa and Acked on 10/30: 
http://marc.info/?t=13831336458r=1w=2.

Roland, I noticed that the Tetsuo's original message didn't cc the linux-rdma 
list?

Mike

 -Original Message-
 From: Stephen Rothwell [mailto:s...@canb.auug.org.au]
 Sent: Sunday, November 03, 2013 11:55 PM
 To: Roland Dreier; linux-r...@vger.kernel.org
 Cc: linux-n...@vger.kernel.org; linux-kernel@vger.kernel.org; Jan Kara;
 Marciniszyn, Mike
 Subject: linux-next: build warning after merge of the infiniband tree
 
 Hi all,
 
 After merging the infiniband tree, today's linux-next build (x86_64
 allmodconfig) produced this warning:
 
 drivers/infiniband/hw/ipath/ipath_user_sdma.c: In function
 'ipath_user_sdma_pin_pages':
 drivers/infiniband/hw/ipath/ipath_user_sdma.c:283:6: warning: 'j' is used
 uninitialized in this function [-Wuninitialized]
   ret = get_user_pages_fast(addr, j, 0, pages);
   ^
 
 Introduced by commit 18fec3c6bdcb (IB/ipath: Convert
 ipath_user_sdma_pin_pages() to use get_user_pages_fast()).  How did that
 pass review or testing?
 
 --
 Cheers,
 Stephen Rothwells...@canb.auug.org.au
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH linux-next-20131029] IB/ipath: Fix random stack overflow.

2013-10-30 Thread Marciniszyn, Mike 
 From: Tetsuo Handa 
> Date: Wed, 30 Oct 2013 16:12:58 +0900
> Subject: [PATCH linux-next-20131029] IB/ipath: Fix random stack overflow.
> Signed-off-by: Tetsuo Handa 
> Cc: 
> ---

Thanks for catching this!

Acked-by: Mike Marciniszyn 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH linux-next-20131029] IB/ipath: Fix random stack overflow.

2013-10-30 Thread Marciniszyn, Mike 
 From: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
 Date: Wed, 30 Oct 2013 16:12:58 +0900
 Subject: [PATCH linux-next-20131029] IB/ipath: Fix random stack overflow.
 Signed-off-by: Tetsuo Handa penguin-ker...@i-love.sakura.ne.jp
 Cc: sta...@vger.kernel.org
 ---

Thanks for catching this!

Acked-by: Mike Marciniszyn mike.marcinis...@intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-07 Thread Marciniszyn, Mike 
> > This patch and the sibling ipath patch will nominally take the mmap_sem
> > twice where the old routine only took it once.   This is a performance
> > issue.
>   It will take mmap_sem only once during normal operation. Only if
> get_user_pages_unlocked() fail, we have to take mmap_sem again to undo
> the change of mm->pinned_vm.
> 
> > Is the intent here to deprecate get_user_pages()?

The old code looked like:
__qib_get_user_pages()
(broken) ulimit test
 for (...)
get_user_pages()

qib_get_user_pages()
mmap_sem lock
__qib_get_user_pages()
 mmap_sem() unlock

The new code is:

get_user_pages_unlocked()
mmap_sem  lock
get_user_pages()
mmap_sem unlock

qib_get_user_pages()
mmap_sem lock
 ulimit test and locked pages maintenance
 mmap_sem unlock
for (...)
get_user_pages_unlocked()

I count an additional pair of mmap_sem transactions in the normal case.

> 
> > Could the lock limit test be pushed into another version of the
> > wrapper so that there is only one set of mmap_sem transactions?
>   I'm sorry, I don't understand what you mean here...
> 

This is what I had in mind:

get_user_pages_ulimit_unlocked()
mmap_sem  lock
ulimit test and locked pages maintenance (from qib/ipath)
 for (...)
   get_user_pages_unlocked()
mmap_sem unlock

qib_get_user_pages()
get_user_pages_ulimit_unlocked()

This really pushes the code into a new wrapper common to ipath/qib and any 
others that might want to combine locking with ulimit enforcement.

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-07 Thread Marciniszyn, Mike 


> -Original Message-
> From: Jan Kara [mailto:j...@suse.cz]
> Sent: Friday, October 04, 2013 2:33 PM
> To: Marciniszyn, Mike
> Cc: Jan Kara; LKML; linux...@kvack.org; infinipath; Roland Dreier; linux-
> r...@vger.kernel.org
> Subject: Re: [PATCH 23/26] ib: Convert qib_get_user_pages() to
> get_user_pages_unlocked()
> 
> On Fri 04-10-13 13:52:49, Marciniszyn, Mike wrote:
> > > Convert qib_get_user_pages() to use get_user_pages_unlocked().  This
> > > shortens the section where we hold mmap_sem for writing and also
> > > removes the knowledge about get_user_pages() locking from ipath
> > > driver. We also fix a bug in testing pinned number of pages when
> changing the code.
> > >
> >
> > This patch and the sibling ipath patch will nominally take the mmap_sem
> > twice where the old routine only took it once.   This is a performance
> > issue.
>   It will take mmap_sem only once during normal operation. Only if
> get_user_pages_unlocked() fail, we have to take mmap_sem again to undo
> the change of mm->pinned_vm.
> 
> > Is the intent here to deprecate get_user_pages()?
>   Well, as much as I'd like to, there are really places in mm code which need
> to call get_user_pages() while holding mmap_sem to be able to inspect
> corresponding vmas etc. So I want to reduce get_user_pages() use as much
> as possible but I'm not really hoping in completely removing it.
> 
> > I agree, the old code's lock limit test is broke and needs to be fixed.
> > I like the elimination of the silly wrapper routine!
> >
> > Could the lock limit test be pushed into another version of the
> > wrapper so that there is only one set of mmap_sem transactions?
>   I'm sorry, I don't understand what you mean here...
> 
>   Honza
> --
> Jan Kara 
> SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-07 Thread Marciniszyn, Mike 


 -Original Message-
 From: Jan Kara [mailto:j...@suse.cz]
 Sent: Friday, October 04, 2013 2:33 PM
 To: Marciniszyn, Mike
 Cc: Jan Kara; LKML; linux...@kvack.org; infinipath; Roland Dreier; linux-
 r...@vger.kernel.org
 Subject: Re: [PATCH 23/26] ib: Convert qib_get_user_pages() to
 get_user_pages_unlocked()
 
 On Fri 04-10-13 13:52:49, Marciniszyn, Mike wrote:
   Convert qib_get_user_pages() to use get_user_pages_unlocked().  This
   shortens the section where we hold mmap_sem for writing and also
   removes the knowledge about get_user_pages() locking from ipath
   driver. We also fix a bug in testing pinned number of pages when
 changing the code.
  
 
  This patch and the sibling ipath patch will nominally take the mmap_sem
  twice where the old routine only took it once.   This is a performance
  issue.
   It will take mmap_sem only once during normal operation. Only if
 get_user_pages_unlocked() fail, we have to take mmap_sem again to undo
 the change of mm-pinned_vm.
 
  Is the intent here to deprecate get_user_pages()?
   Well, as much as I'd like to, there are really places in mm code which need
 to call get_user_pages() while holding mmap_sem to be able to inspect
 corresponding vmas etc. So I want to reduce get_user_pages() use as much
 as possible but I'm not really hoping in completely removing it.
 
  I agree, the old code's lock limit test is broke and needs to be fixed.
  I like the elimination of the silly wrapper routine!
 
  Could the lock limit test be pushed into another version of the
  wrapper so that there is only one set of mmap_sem transactions?
   I'm sorry, I don't understand what you mean here...
 
   Honza
 --
 Jan Kara j...@suse.cz
 SUSE Labs, CR
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-07 Thread Marciniszyn, Mike 
  This patch and the sibling ipath patch will nominally take the mmap_sem
  twice where the old routine only took it once.   This is a performance
  issue.
   It will take mmap_sem only once during normal operation. Only if
 get_user_pages_unlocked() fail, we have to take mmap_sem again to undo
 the change of mm-pinned_vm.
 
  Is the intent here to deprecate get_user_pages()?

The old code looked like:
__qib_get_user_pages()
(broken) ulimit test
 for (...)
get_user_pages()

qib_get_user_pages()
mmap_sem lock
__qib_get_user_pages()
 mmap_sem() unlock

The new code is:

get_user_pages_unlocked()
mmap_sem  lock
get_user_pages()
mmap_sem unlock

qib_get_user_pages()
mmap_sem lock
 ulimit test and locked pages maintenance
 mmap_sem unlock
for (...)
get_user_pages_unlocked()

I count an additional pair of mmap_sem transactions in the normal case.

 
  Could the lock limit test be pushed into another version of the
  wrapper so that there is only one set of mmap_sem transactions?
   I'm sorry, I don't understand what you mean here...
 

This is what I had in mind:

get_user_pages_ulimit_unlocked()
mmap_sem  lock
ulimit test and locked pages maintenance (from qib/ipath)
 for (...)
   get_user_pages_unlocked()
mmap_sem unlock

qib_get_user_pages()
get_user_pages_ulimit_unlocked()

This really pushes the code into a new wrapper common to ipath/qib and any 
others that might want to combine locking with ulimit enforcement.

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-04 Thread Marciniszyn, Mike 
> Convert qib_get_user_pages() to use get_user_pages_unlocked().  This
> shortens the section where we hold mmap_sem for writing and also removes
> the knowledge about get_user_pages() locking from ipath driver. We also fix
> a bug in testing pinned number of pages when changing the code.
> 

This patch and the sibling ipath patch will nominally take the mmap_sem twice 
where the old routine only took it once.   This is a performance issue.

Is the intent here to deprecate get_user_pages()?

I agree, the old code's lock limit test is broke and needs to be fixed.   I 
like the elimination of the silly wrapper routine!

Could the lock limit test be pushed into another version of the wrapper so that 
there is only one set of mmap_sem transactions?

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-04 Thread Marciniszyn, Mike 
Inadvertent send!

Mike

> -Original Message-
> From: Marciniszyn, Mike
> Sent: Friday, October 04, 2013 9:39 AM
> To: Jan Kara
> Cc: LKML; linux...@kvack.org; infinipath; Roland Dreier; linux-
> r...@vger.kernel.org
> Subject: RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to
> get_user_pages_unlocked()
> 
> 
> 
> > -Original Message-
> > From: Jan Kara [mailto:j...@suse.cz]
> > Sent: Wednesday, October 02, 2013 11:39 AM
> > To: Marciniszyn, Mike
> > Cc: Jan Kara; LKML; linux...@kvack.org; infinipath; Roland Dreier;
> > linux- r...@vger.kernel.org
> > Subject: Re: [PATCH 23/26] ib: Convert qib_get_user_pages() to
> > get_user_pages_unlocked()
> >
> > On Wed 02-10-13 15:32:47, Marciniszyn, Mike wrote:
> > > > > The risk of GUP fast is the loss of the "force" arg on GUP fast,
> > > > > which I don't see as significant give our use case.
> > > >   Yes. I was discussing with Roland some time ago whether the
> > > > force argument is needed and he said it is. So I kept the
> > > > arguments of
> > > > get_user_pages() intact and just simplified the locking...
> > >
> > > The PSM side of the code is a more traditional use of GUP (like
> > > direct I/O), so I think it is a different use case than the locking
> > > for IB memory regions.
> >   Ah, I see. Whatever suits you best. I don't really care as long as
> > get_user_pages() locking doesn't leak into IB drivers :)
> >
> > Honza
> > --
> > Jan Kara 
> > SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-04 Thread Marciniszyn, Mike 
> The PSM side of the code is a more traditional use of GUP (like direct I/O), 
> so
> I think it is a different use case than the locking for IB memory regions.

I have resubmitted the two deadlock fixes using get_user_pages_fast() and 
marked them stable.

See http://marc.info/?l=linux-rdma=138089335506355=2

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-04 Thread Marciniszyn, Mike 


> -Original Message-
> From: Jan Kara [mailto:j...@suse.cz]
> Sent: Wednesday, October 02, 2013 11:39 AM
> To: Marciniszyn, Mike
> Cc: Jan Kara; LKML; linux...@kvack.org; infinipath; Roland Dreier; linux-
> r...@vger.kernel.org
> Subject: Re: [PATCH 23/26] ib: Convert qib_get_user_pages() to
> get_user_pages_unlocked()
> 
> On Wed 02-10-13 15:32:47, Marciniszyn, Mike wrote:
> > > > The risk of GUP fast is the loss of the "force" arg on GUP fast,
> > > > which I don't see as significant give our use case.
> > >   Yes. I was discussing with Roland some time ago whether the force
> > > argument is needed and he said it is. So I kept the arguments of
> > > get_user_pages() intact and just simplified the locking...
> >
> > The PSM side of the code is a more traditional use of GUP (like direct
> > I/O), so I think it is a different use case than the locking for IB
> > memory regions.
>   Ah, I see. Whatever suits you best. I don't really care as long as
> get_user_pages() locking doesn't leak into IB drivers :)
> 
>   Honza
> --
> Jan Kara 
> SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-04 Thread Marciniszyn, Mike 


 -Original Message-
 From: Jan Kara [mailto:j...@suse.cz]
 Sent: Wednesday, October 02, 2013 11:39 AM
 To: Marciniszyn, Mike
 Cc: Jan Kara; LKML; linux...@kvack.org; infinipath; Roland Dreier; linux-
 r...@vger.kernel.org
 Subject: Re: [PATCH 23/26] ib: Convert qib_get_user_pages() to
 get_user_pages_unlocked()
 
 On Wed 02-10-13 15:32:47, Marciniszyn, Mike wrote:
The risk of GUP fast is the loss of the force arg on GUP fast,
which I don't see as significant give our use case.
 Yes. I was discussing with Roland some time ago whether the force
   argument is needed and he said it is. So I kept the arguments of
   get_user_pages() intact and just simplified the locking...
 
  The PSM side of the code is a more traditional use of GUP (like direct
  I/O), so I think it is a different use case than the locking for IB
  memory regions.
   Ah, I see. Whatever suits you best. I don't really care as long as
 get_user_pages() locking doesn't leak into IB drivers :)
 
   Honza
 --
 Jan Kara j...@suse.cz
 SUSE Labs, CR
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()

2013-10-04 Thread Marciniszyn, Mike 
 The PSM side of the code is a more traditional use of GUP (like direct I/O), 
 so
 I think it is a different use case than the locking for IB memory regions.

I have resubmitted the two deadlock fixes using get_user_pages_fast() and 
marked them stable.

See http://marc.info/?l=linux-rdmam=138089335506355w=2

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

  1   2   >