Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-07 Thread Jack Morgenstein
On Wednesday 03 July 2013 22:26, Roland Dreier wrote: Look at the actual timer code.  del_timer_sync() won't work if something unrelated re-adds the timer, but it will work if the timer itself is what re-adds itself. Documentation/DocBook/kernel-locking.tmpl says:       Another common

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-03 Thread Or Gerlitz
On 01/07/2013 20:49, Roland Dreier wrote: - I think the active flag for the health check timer is unnecessary. It can just be stopped with del_timer_sync(). Hi Roland Jack looked on this comment/code and he says that the active flag is used to prevent re-scheduling the timer from inside the

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-03 Thread Roland Dreier
On Wed, Jul 3, 2013 at 9:41 AM, Or Gerlitz ogerl...@mellanox.com wrote: Jack looked on this comment/code and he says that the active flag is used to prevent re-scheduling the timer from inside the timer handling routine. In the kernel, the comment header in the source file for del_timer_sync

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-03 Thread Or Gerlitz
On Wed, Jul 3, 2013 at 10:26 PM, Roland Dreier rol...@kernel.org wrote: On Wed, Jul 3, 2013 at 9:41 AM, Or Gerlitz ogerl...@mellanox.com wrote: Jack looked on this comment/code and he says that the active flag is used to prevent re-scheduling the timer from inside the timer handling routine.

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-02 Thread Or Gerlitz
On Tue, Jul 2, 2013 at 12:22 AM, Roland Dreier rol...@kernel.org wrote: Also, sparse warns about [...] in mlx5_ib.h. Nor does it have any callers, so it's a bit hard to tell if it's really and truly a bug. removing this function for V2 -- To unsubscribe from this list: send the line

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-02 Thread Or Gerlitz
On Mon, Jul 1, 2013 at 9:03 PM, Roland Dreier rol...@kernel.org wrote: In general I don't think overriding the CFLAGS (as you do in the mlx5 Makefiles) is a good idea, and in particular here your -Wall -Werror break the build, at least for my gcc 4.7.3: CC

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-02 Thread Or Gerlitz
On Mon, Jul 1, 2013 at 8:49 PM, Roland Dreier rol...@kernel.org wrote: So I'm inclined to apply the mlx5 driver for 3.11, since it's a completely new driver. However, reading through it so far I had the following comments, and I'd like these cleanups addressed along with Dave Miller's:

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-01 Thread Roland Dreier
In general I don't think overriding the CFLAGS (as you do in the mlx5 Makefiles) is a good idea, and in particular here your -Wall -Werror break the build, at least for my gcc 4.7.3: CC drivers/infiniband/hw/mlx5/qp.o /home/roland/Src/linux-merge.git/drivers/infiniband/hw/mlx5/qp.c: In

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-01 Thread Roland Dreier
On Mon, Jul 1, 2013 at 11:03 AM, Joe Perches j...@perches.com wrote: There's some value in block enabling/disabling messages that dynamic_debug doesn't currently offer. As far as I can see, the mlx5 stuff ends up being per-file. Which dynamic debug already does offer. - R. -- To unsubscribe

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-01 Thread Roland Dreier
Also, sparse warns about the following. It seems there's an endianness bug in int mlx5_ib_umem_populate_pas(struct mlx5_ib_dev *dev, struct ib_umem *umem, int npages, u64 *pas) in mem.c, since it doesn't match what void mlx5_ib_populate_pas(struct mlx5_ib_dev *dev, struct

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-07-01 Thread Joe Perches
On Mon, 2013-07-01 at 14:20 -0700, Roland Dreier wrote: On Mon, Jul 1, 2013 at 1:19 PM, Joe Perches j...@perches.com wrote: I think these are the groupings. +enum { + MLX5_MOD_MAIN, + MLX5_MOD_CMDIF, + MLX5_MOD_EQ, + MLX5_MOD_QP, + MLX5_MOD_PGALLOC,

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-06-28 Thread Or Gerlitz
On Sun, Jun 16, 2013 at 3:02 PM, Eli Cohen e...@dev.mellanox.co.il wrote: From: Eli Cohen e...@mellanox.com The patches that follow constitute the driver for Mellanox's 5th generation of HCAs named Connect-IB. The driver is comprised of two kernel modules: mlx5_ib and mlx5_core. This

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-06-28 Thread Or Gerlitz
On Fri, Jun 28, 2013 at 4:20 PM, Or Gerlitz or.gerl...@gmail.com wrote: On Sun, Jun 16, 2013 at 3:02 PM, Eli Cohen e...@dev.mellanox.co.il wrote: From: Eli Cohen e...@mellanox.com The patches that follow constitute the driver for Mellanox's 5th generation of HCAs named Connect-IB. The

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-06-25 Thread David Miller
From: Or Gerlitz ogerl...@mellanox.com Date: Thu, 20 Jun 2013 17:44:49 +0300 So we skipped netdev in V0, in an attempt to reduce cross postings... anyway, the mlx5_core driver is similar story as of mlx4_core. So, if looking forward, for the initial merge to be simpler, are you OK for both

Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

2013-06-20 Thread Or Gerlitz
On 16/06/2013 15:02, Eli Cohen wrote: From: Eli Cohen e...@mellanox.com The patches that follow constitute the driver for Mellanox's 5th generation of HCAs named Connect-IB. The driver is comprised of two kernel modules: mlx5_ib and mlx5_core. This partitioning resembles what we have for mlx4