Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
On Fri, Dec 25, 2015 at 5:56 PM, Hal Rosenstock wrote:
> On 12/25/2015 9:50 AM, Hal Rosenstock wrote:
>> On 12/24/2015 11:09 AM, Matan Barak wrote:
>>> On Thu, Dec 24, 2015 at 4:07 PM, Matan Barak
>>> wrote:
On Thu, Dec 24, 2015 at 2:38 PM, Or Gerlitz wrote:
> On 12/24/2015 12:42 PM, Sagi Grimberg wrote:
>>
>>
This patch seems to generate a list corruption [1] when I test
with Doug's for-4.5 tree. Eran, care to take a look at this?
>>>
>>>
>>> This patch is part from a series that was introduced in 4.3-rc1 [1],
>>
>>
>> Then something else broke it. Can people check their patches on doug's
>> tree? At the moment it's unusable...
>
Leon and I have checked Doug's tree with mlx4_ib disabled and we
didn't encounter any error.
We ran ucmatose over IB connection (in mlx5) and it worked flawlessly.
>
> Yes, I checked the branch up to commit 882f3b3 "Merge branches
> '4.5/Or-cleanup' and '4.5/rdma-cq' into k.o/for-4.5" and it works (rping,
> ibv_rc_pingpong over top of mlx4 VPI)
>
>>>
>>> Regarding mlx4, Eran and I analyzed it. We didn't test that, but it
>>> seems like the bug is introduced in the 64bit counters test. Here's a
>>> proposal:
>>>
>>> diff --git a/drivers/infiniband/core/sysfs.c
>>> b/drivers/infiniband/core/sysfs.c
>>> index 539040f..8da3c83 100644
>>> --- a/drivers/infiniband/core/sysfs.c
>>> +++ b/drivers/infiniband/core/sysfs.c
>>> @@ -714,11 +714,12 @@ err:
>>> * Figure out which counter table to use depending on
>>> * the device capabilities.
>>> */
>>> -static struct attribute_group *get_counter_table(struct ib_device *dev)
>>> +static struct attribute_group *get_counter_table(struct ib_device *dev,
>>> + int port_num)
>>> {
>>> struct ib_class_port_info cpi;
>>>
>>> - if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,
>>> + if (get_perf_mad(dev, port_num, IB_PMA_CLASS_PORT_INFO,
>>> &cpi, 40, sizeof(cpi)) >= 0) {
>>
>> Your proposal is similar to earlier version of Christoph's patch but was
>> changed since ClassPortInfo attribute does not have PortSelect field
>> like other PerfMgt attributes which is where this port num would be
>> placed. In ClassPortInfo attribute, that location would be the
>> ClassVersion field that would be set to port number in PerfMgt Get query.
>
> In actuality, I don't think it really matters as this is a Get not a Set
> and the PMA would do the right thing even if some field in the CPI were
> stepped on.
>
Well, it does matter as it calls the vendor driver with port_num = 0.
Since the kernel is trusted, the vendor driver expects a valid port number.
Giving it an invalid number might result in memory corruptions, as
demonstrated in this case.
>> -- Hal
Matan
>>
>>>
>>> if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
>>> @@ -776,7 +777,7 @@ static int add_port(struct ib_device *device, int
>>> port_num,
>>> goto err_put;
>>> }
>>>
>>> - p->pma_table = get_counter_table(device);
>>> + p->pma_table = get_counter_table(device, port_num);
>>> ret = sysfs_create_group(&p->kobj, p->pma_table);
>>> if (ret)
>>> goto err_put_gid_attrs;
>>>
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
On 12/25/2015 9:50 AM, Hal Rosenstock wrote:
> On 12/24/2015 11:09 AM, Matan Barak wrote:
>> On Thu, Dec 24, 2015 at 4:07 PM, Matan Barak
>> wrote:
>>> On Thu, Dec 24, 2015 at 2:38 PM, Or Gerlitz wrote:
On 12/24/2015 12:42 PM, Sagi Grimberg wrote:
>
>
>>> This patch seems to generate a list corruption [1] when I test
>>> with Doug's for-4.5 tree. Eran, care to take a look at this?
>>
>>
>> This patch is part from a series that was introduced in 4.3-rc1 [1],
>
>
> Then something else broke it. Can people check their patches on doug's
> tree? At the moment it's unusable...
>>>
>>> Leon and I have checked Doug's tree with mlx4_ib disabled and we
>>> didn't encounter any error.
>>> We ran ucmatose over IB connection (in mlx5) and it worked flawlessly.
>>>
Yes, I checked the branch up to commit 882f3b3 "Merge branches
'4.5/Or-cleanup' and '4.5/rdma-cq' into k.o/for-4.5" and it works (rping,
ibv_rc_pingpong over top of mlx4 VPI)
>>
>> Regarding mlx4, Eran and I analyzed it. We didn't test that, but it
>> seems like the bug is introduced in the 64bit counters test. Here's a
>> proposal:
>>
>> diff --git a/drivers/infiniband/core/sysfs.c
>> b/drivers/infiniband/core/sysfs.c
>> index 539040f..8da3c83 100644
>> --- a/drivers/infiniband/core/sysfs.c
>> +++ b/drivers/infiniband/core/sysfs.c
>> @@ -714,11 +714,12 @@ err:
>> * Figure out which counter table to use depending on
>> * the device capabilities.
>> */
>> -static struct attribute_group *get_counter_table(struct ib_device *dev)
>> +static struct attribute_group *get_counter_table(struct ib_device *dev,
>> + int port_num)
>> {
>> struct ib_class_port_info cpi;
>>
>> - if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,
>> + if (get_perf_mad(dev, port_num, IB_PMA_CLASS_PORT_INFO,
>> &cpi, 40, sizeof(cpi)) >= 0) {
>
> Your proposal is similar to earlier version of Christoph's patch but was
> changed since ClassPortInfo attribute does not have PortSelect field
> like other PerfMgt attributes which is where this port num would be
> placed. In ClassPortInfo attribute, that location would be the
> ClassVersion field that would be set to port number in PerfMgt Get query.
In actuality, I don't think it really matters as this is a Get not a Set
and the PMA would do the right thing even if some field in the CPI were
stepped on.
> -- Hal
>
>>
>> if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
>> @@ -776,7 +777,7 @@ static int add_port(struct ib_device *device, int
>> port_num,
>> goto err_put;
>> }
>>
>> - p->pma_table = get_counter_table(device);
>> + p->pma_table = get_counter_table(device, port_num);
>> ret = sysfs_create_group(&p->kobj, p->pma_table);
>> if (ret)
>> goto err_put_gid_attrs;
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
On 12/24/2015 11:09 AM, Matan Barak wrote:
> On Thu, Dec 24, 2015 at 4:07 PM, Matan Barak
> wrote:
>> On Thu, Dec 24, 2015 at 2:38 PM, Or Gerlitz wrote:
>>> On 12/24/2015 12:42 PM, Sagi Grimberg wrote:
>> This patch seems to generate a list corruption [1] when I test
>> with Doug's for-4.5 tree. Eran, care to take a look at this?
>
>
> This patch is part from a series that was introduced in 4.3-rc1 [1],
Then something else broke it. Can people check their patches on doug's
tree? At the moment it's unusable...
>>>
>>
>> Leon and I have checked Doug's tree with mlx4_ib disabled and we
>> didn't encounter any error.
>> We ran ucmatose over IB connection (in mlx5) and it worked flawlessly.
>>
>>>
>>> Yes, I checked the branch up to commit 882f3b3 "Merge branches
>>> '4.5/Or-cleanup' and '4.5/rdma-cq' into k.o/for-4.5" and it works (rping,
>>> ibv_rc_pingpong over top of mlx4 VPI)
>>>
>
> Regarding mlx4, Eran and I analyzed it. We didn't test that, but it
> seems like the bug is introduced in the 64bit counters test. Here's a
> proposal:
>
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 539040f..8da3c83 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -714,11 +714,12 @@ err:
> * Figure out which counter table to use depending on
> * the device capabilities.
> */
> -static struct attribute_group *get_counter_table(struct ib_device *dev)
> +static struct attribute_group *get_counter_table(struct ib_device *dev,
> + int port_num)
> {
> struct ib_class_port_info cpi;
>
> - if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,
> + if (get_perf_mad(dev, port_num, IB_PMA_CLASS_PORT_INFO,
> &cpi, 40, sizeof(cpi)) >= 0) {
Your proposal is similar to earlier version of Christoph's patch but was
changed since ClassPortInfo attribute does not have PortSelect field
like other PerfMgt attributes which is where this port num would be
placed. In ClassPortInfo attribute, that location would be the
ClassVersion field that would be set to port number in PerfMgt Get query.
-- Hal
>
> if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
> @@ -776,7 +777,7 @@ static int add_port(struct ib_device *device, int
> port_num,
> goto err_put;
> }
>
> - p->pma_table = get_counter_table(device);
> + p->pma_table = get_counter_table(device, port_num);
> ret = sysfs_create_group(&p->kobj, p->pma_table);
> if (ret)
> goto err_put_gid_attrs;
>
>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
On Thu, Dec 24, 2015 at 4:07 PM, Matan Barak wrote:
> On Thu, Dec 24, 2015 at 2:38 PM, Or Gerlitz wrote:
>> On 12/24/2015 12:42 PM, Sagi Grimberg wrote:
>>>
>>>
> This patch seems to generate a list corruption [1] when I test
> with Doug's for-4.5 tree. Eran, care to take a look at this?
This patch is part from a series that was introduced in 4.3-rc1 [1],
>>>
>>>
>>> Then something else broke it. Can people check their patches on doug's
>>> tree? At the moment it's unusable...
>>
>
> Leon and I have checked Doug's tree with mlx4_ib disabled and we
> didn't encounter any error.
> We ran ucmatose over IB connection (in mlx5) and it worked flawlessly.
>
>>
>> Yes, I checked the branch up to commit 882f3b3 "Merge branches
>> '4.5/Or-cleanup' and '4.5/rdma-cq' into k.o/for-4.5" and it works (rping,
>> ibv_rc_pingpong over top of mlx4 VPI)
>>
Regarding mlx4, Eran and I analyzed it. We didn't test that, but it
seems like the bug is introduced in the 64bit counters test. Here's a
proposal:
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 539040f..8da3c83 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -714,11 +714,12 @@ err:
* Figure out which counter table to use depending on
* the device capabilities.
*/
-static struct attribute_group *get_counter_table(struct ib_device *dev)
+static struct attribute_group *get_counter_table(struct ib_device *dev,
+ int port_num)
{
struct ib_class_port_info cpi;
- if (get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO,
+ if (get_perf_mad(dev, port_num, IB_PMA_CLASS_PORT_INFO,
&cpi, 40, sizeof(cpi)) >= 0) {
if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
@@ -776,7 +777,7 @@ static int add_port(struct ib_device *device, int port_num,
goto err_put;
}
- p->pma_table = get_counter_table(device);
+ p->pma_table = get_counter_table(device, port_num);
ret = sysfs_create_group(&p->kobj, p->pma_table);
if (ret)
goto err_put_gid_attrs;
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
On Thu, Dec 24, 2015 at 2:38 PM, Or Gerlitz wrote: > On 12/24/2015 12:42 PM, Sagi Grimberg wrote: >> >> This patch seems to generate a list corruption [1] when I test with Doug's for-4.5 tree. Eran, care to take a look at this? >>> >>> >>> This patch is part from a series that was introduced in 4.3-rc1 [1], >> >> >> Then something else broke it. Can people check their patches on doug's >> tree? At the moment it's unusable... > Leon and I have checked Doug's tree with mlx4_ib disabled and we didn't encounter any error. We ran ucmatose over IB connection (in mlx5) and it worked flawlessly. > > Yes, I checked the branch up to commit 882f3b3 "Merge branches > '4.5/Or-cleanup' and '4.5/rdma-cq' into k.o/for-4.5" and it works (rping, > ibv_rc_pingpong over top of mlx4 VPI) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
On 12/24/2015 12:42 PM, Sagi Grimberg wrote: This patch seems to generate a list corruption [1] when I test with Doug's for-4.5 tree. Eran, care to take a look at this? This patch is part from a series that was introduced in 4.3-rc1 [1], Then something else broke it. Can people check their patches on doug's tree? At the moment it's unusable... Yes, I checked the branch up to commit 882f3b3 "Merge branches '4.5/Or-cleanup' and '4.5/rdma-cq' into k.o/for-4.5" and it works (rping, ibv_rc_pingpong over top of mlx4 VPI) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
This patch seems to generate a list corruption [1] when I test with Doug's for-4.5 tree. Eran, care to take a look at this? This patch is part from a series that was introduced in 4.3-rc1 [1], Then something else broke it. Can people check their patches on doug's tree? At the moment it's unusable... -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
On 12/24/2015 12:12 PM, Sagi Grimberg wrote: This patch seems to generate a list corruption [1] when I test with Doug's for-4.5 tree. Eran, care to take a look at this? This patch is part from a series that was introduced in 4.3-rc1 [1], did 4.4-rc5/6 worked for you before you uploaded there further patches? Or. [1] fbfb662 IB/mlx4: Add support for blocking multicast loopback QP creation user flag 7b59f0f IB/mlx4: Add counter based implementation for QP multicast loopback block 3ba8e31 IB/mlx4: Add IB counters table 74194fb net/mlx4_en: Implement mcast loopback prevention for ETH qps 9a89283 net/mlx4_core: Add support for filtering multicast loopback ddf9529 IB/core: Allow setting create flags in QP init attribute 6d8a749 IB/core: Extend ib_uverbs_create_qp -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
Doug, I'm also can't load mlx5 drivers in your tree [1] but I don't know where it's from, it can come from pretty much everything... Now I'm left with no useable HW to test with :( [1]: mlx5_core :06:00.0: firmware version: 12.14.74 mlx5_core :06:00.1: firmware version: 12.14.74 mlx5_ib: Mellanox Connect-IB Infiniband driver v2.2-1 (Feb 2014) command failed, status bad parameter(0x3), syndrome 0x7424da command failed, status bad parameter(0x3), syndrome 0x7424da -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 for-next 5/7] IB/mlx4: Add IB counters table
This patch seems to generate a list corruption [1] when I test with Doug's for-4.5 tree. Eran, care to take a look at this? [1]: mlx4_core: Mellanox ConnectX core driver v2.2-1 (Feb, 2014) mlx4_core: Initializing :04:00.0 mlx4_core :04:00.0: PCIe link speed is 8.0GT/s, device supports 8.0GT/s mlx4_core :04:00.0: PCIe link width is x8, device supports x8 mlx4_ib_add: mlx4_ib: Mellanox ConnectX InfiniBand driver v2.2-1 (Feb 2014) mlx4_ib_add: counter index 0 for port 1 allocated 0 mlx4_ib_add: counter index 1 for port 2 allocated 0 BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] __list_add+0x26/0xd0 PGD 46da14067 PUD 46daa0067 PMD 0 Oops: [#1] SMP Modules linked in: mlx4_ib(+) ib_sa ib_mad mlx4_core mlx5_ib mlx5_core ib_core ib_addr netconsole configfs nfsv3 nfs fscache cfg80211 rfkill x86_pkg_temp_thermal coretemp kvm_intel kvm irqbypass crc32c_intel aesni_intel aes_x86_64 glue_helper lrw dm_mod gf128mul ablk_helper cryptd iTCO_wdt iTCO_vendor_support sb_edac shpchp ipmi_si ioatdma lpc_ich mfd_core edac_core pcspkr wmi ipmi_msghandler i2c_i801 acpi_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables ext4 mbcache jbd2 sd_mod isci libsas igb serio_raw ahci ptp pps_core libahci i2c_algo_bit scsi_transport_sas i2c_core dca ipv6 autofs4 [last unloaded: mlx5_core] CPU: 0 PID: 1737 Comm: modprobe Not tainted 4.4.0-rc6+ #107 Hardware name: Supermicro SYS-1027R-WRF/X9DRW, BIOS 3.0a 08/08/2013 task: 8804673da800 ti: 880466694000 task.ti: 880466694000 RIP: 0010:[] [] __list_add+0x26/0xd0 RSP: 0018:880466697898 EFLAGS: 00010246 RAX: RBX: 8804666978c8 RCX: 8804673da800 RDX: 88086b8539b8 RSI: RDI: 8804666978c8 RBP: 8804666978b8 R08: R09: 0001 R10: R11: fffe R12: 88086b8539b8 R13: R14: 88086b8539b8 R15: 880466697908 FS: 7f37a02cf700() GS:88047fc0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 00046b6ee000 CR4: 000406f0 Stack: 8804673da800 88086b8539b0 8804673da800 88086b8539b4 880466697958 8154f7be 880466697904 0292 880466697938 81259bc1 7f49 824000c0 Call Trace: [] __mutex_lock_slowpath+0x6e/0x110 [] ? ida_simple_get+0x91/0x100 [] ? kernfs_next_descendant_post+0x1e/0x90 [] ? kernfs_activate+0x86/0xf0 [] mutex_lock+0x1e/0x40 [] iboe_process_mad+0x73/0x180 [mlx4_ib] [] mlx4_ib_process_mad+0xd6/0x110 [mlx4_ib] [] get_perf_mad+0x103/0x140 [ib_core] [] get_counter_table+0x24/0x40 [ib_core] [] ? __kmalloc+0xde/0xe0 [] add_port+0x115/0x3f0 [ib_core] [] ib_device_register_sysfs+0xee/0x160 [ib_core] [] ib_register_device+0x1d5/0x300 [ib_core] [] mlx4_ib_add+0x78b/0xd00 [mlx4_ib] [] mlx4_add_device+0x3e/0xb0 [mlx4_core] [] mlx4_register_interface+0x87/0xe0 [mlx4_core] [] mlx4_ib_init+0x55/0x72 [mlx4_ib] [] ? 0xa0096000 [] do_one_initcall+0xa8/0x1c0 [] do_init_module+0x5f/0x210 [] load_module+0x5d7/0x700 [] ? mod_sysfs_teardown+0x140/0x140 [] ? module_sect_show+0x20/0x20 [] SyS_finit_module+0xbb/0xf0 [] entry_SYSCALL_64_fastpath+0x12/0x6a Code: 90 90 90 90 90 55 48 89 e5 48 83 ec 20 48 89 5d e8 4c 89 65 f0 48 89 fb 4c 89 6d f8 4c 8b 42 08 49 89 f5 49 89 d4 49 39 f0 75 31 <4d> 8b 45 00 4d 39 c4 75 6f 4c 39 e3 74 45 4c 39 eb 74 40 49 89 RIP [] __list_add+0x26/0xd0 RSP CR2: ---[ end trace 5f4fe0ca857661e6 ]--- -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
