| Message ID | CAJ3xEMjPZYgLhDi_6Jxkx_dg_RaehD=3SchMt_noLzQEChPrxQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
On 12/08/2016 01:26 AM, Or Gerlitz wrote: > Matan came up with a fix (below) which solves all these sparse > complaints in both drivers (mlx5 IB and core) > while basically not touching the code, we will be going with that fix > to 4.11, thanks, I will add reported > by pointing to you. Hello Or, Please consider to keep the following change: - MLX5_SET(mkc, mkc, en_rinval, !!((type == IB_MW_TYPE_2))); + MLX5_SET(mkc, mkc, en_rinval, type == IB_MW_TYPE_2); I don't think that specifying two boolean not (!) operators in front of a boolean expression is useful :-) Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 08, 2016 at 11:26:32AM +0200, Or Gerlitz wrote: > /* insert a value to a struct */ > #define MLX5_SET(typ, p, fld, v) do { \ > + typeof( v ) _v = v; \ > BUILD_BUG_ON(__mlx5_st_sz_bits(typ) % 32); \ > *((__be32 *)(p) + __mlx5_dw_off(typ, fld)) = \ > cpu_to_be32((be32_to_cpu(*((__be32 *)(p) + __mlx5_dw_off(typ, fld))) & \ > - (~__mlx5_dw_mask(typ, fld))) | (((v) & > __mlx5_mask(typ, fld)) \ > + (~__mlx5_dw_mask(typ, fld))) | (((_v) & > __mlx5_mask(typ, fld)) \ > << __mlx5_dw_bit_off(typ, fld))); \ > } while (0) Ugh, that is ugly. This is better, it has proper type safety :| static inline _mlx5_set(__be32 *p, u32 dw_mask, u32 mask, u32 dw_off, unsigned int bit_off, u32 val) { *(p + dw_off) = cpu_to_b32((be32_to_cpu_p(p + dw_off) & ~dw_mask) | ((val & mask) << bit_off)); } #define MLX5_SET(typ, p, fld, v) \ _mlx5_set(p, __mlx5_dw_mask(typ, fld), __mlx5_mask(typ, fld), __mlx5_dw_off(typ, fld), __mlx5_dw_bit_off(typ, fld), v) Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/8/2016 12:53 PM, Jason Gunthorpe wrote: > On Thu, Dec 08, 2016 at 11:26:32AM +0200, Or Gerlitz wrote: >> /* insert a value to a struct */ >> #define MLX5_SET(typ, p, fld, v) do { \ >> + typeof( v ) _v = v; \ >> BUILD_BUG_ON(__mlx5_st_sz_bits(typ) % 32); \ >> *((__be32 *)(p) + __mlx5_dw_off(typ, fld)) = \ >> cpu_to_be32((be32_to_cpu(*((__be32 *)(p) + __mlx5_dw_off(typ, fld))) & \ >> - (~__mlx5_dw_mask(typ, fld))) | (((v) & >> __mlx5_mask(typ, fld)) \ >> + (~__mlx5_dw_mask(typ, fld))) | (((_v) & >> __mlx5_mask(typ, fld)) \ >> << __mlx5_dw_bit_off(typ, fld))); \ >> } while (0) > > Ugh, that is ugly. > > This is better, it has proper type safety :| > > static inline _mlx5_set(__be32 *p, u32 dw_mask, > u32 mask, u32 dw_off, > unsigned int bit_off, u32 val) > { > *(p + dw_off) = cpu_to_b32((be32_to_cpu_p(p + dw_off) & > ~dw_mask) | ((val & mask) << bit_off)); > } > > #define MLX5_SET(typ, p, fld, v) \ > _mlx5_set(p, __mlx5_dw_mask(typ, fld), > __mlx5_mask(typ, fld), > __mlx5_dw_off(typ, fld), > __mlx5_dw_bit_off(typ, fld), v) > > Jason > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Dropped this patch and will wait for fixed up version from Mellanox.
--- a/include/linux/mlx5/device.h +++ b/include/linux/mlx5/device.h @@ -67,10 +67,11 @@ /* insert a value to a struct */ #define MLX5_SET(typ, p, fld, v) do { \ + typeof( v ) _v = v; \ BUILD_BUG_ON(__mlx5_st_sz_bits(typ) % 32); \ *((__be32 *)(p) + __mlx5_dw_off(typ, fld)) = \ cpu_to_be32((be32_to_cpu(*((__be32 *)(p) + __mlx5_dw_off(typ, fld))) & \ - (~__mlx5_dw_mask(typ, fld))) | (((v) & __mlx5_mask(typ, fld)) \ + (~__mlx5_dw_mask(typ, fld))) | (((_v) & __mlx5_mask(typ, fld)) \ << __mlx5_dw_bit_off(typ, fld))); \
On Tue, Dec 6, 2016 at 3:19 AM, Bart Van Assche <bart.vanassche@sandisk.com> wrote: > This patch does not change any functionality. Hi Bart, Matan came up with a fix (below) which solves all these sparse complaints in both drivers (mlx5 IB and core) while basically not touching the code, we will be going with that fix to 4.11, thanks, I will add reported by pointing to you. Doug, in case you picked that, I guess you have to drop it. Or. iff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h index 9f48936..0abdc66 100644 } while (0) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html