RE: [RFC PATCH 00/16] ib_mad: Add support for Intel Omni-Path Architecture (OPA) MAD processing.

["Weiny, Ira" <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3097 bytes --]

> >
> >>>    [RFC PATCH 07/16] ib/mad: create a jumbo MAD kmem_cache
> >> why not use a single kmem-cache instance with a non hard coded
> >> element size, 256B (or whatever we use today) or 2KB?
> > I wanted to be able to adjust the element count of the caches separately to
> better tune overall memory usage.  However, I stopped short of adding
> additional module parameters to adjust the 2K cache at this time.
> 
> 
> I tend to think that the resulted code is too much of a special purpose one
> under a  (jumbo == 2K) assumption. See some more comments in the individual
> patches and we'll take it from there.
> 

Ok, I'll address those comments in the other email threads.

> 
> 
> >
> >> Also (nit), please change the prefix for all patches to be IB/mad:
> >> and not
> >> ib/mad: to comply with the existing habit of patch titles for the IB
> >> subsystem
> > I will thanks.
> 
> Good. See below another easy-to-fix nitpicking comment, but before that, for
> the sake of easier review and post-robustness of the code to future bisections,
> please do a re-ordering of the series such that all general refactoring and pre-
> patches come before the OPApatches.
> 
> This goes to re-order the current series such tat patches 8/9/10 are located
> after patch 14, as listed here:
> 
>    [RFC PATCH 01/16] ib/mad: rename is_data_mad to is_rmpp_data_mad
>    [RFC PATCH 02/16] ib/core: add IB_DEVICE_JUMBO_MAD_SUPPORT device
> cap
>    [RFC PATCH 03/16] ib/mad: Add check for jumbo MADs support on a device
>    [RFC PATCH 04/16] ib/mad: add base version parameter to
>    [RFC PATCH 05/16] ib/mad: Add MAD size parameters to process_mad
>    [RFC PATCH 06/16] ib/mad: Create jumbo_mad data structures
>    [RFC PATCH 07/16] ib/mad: create a jumbo MAD kmem_cache
>    [RFC PATCH 11/16] ib/mad: create helper function for
>    [RFC PATCH 12/16] ib/mad: create helper function for
>    [RFC PATCH 13/16] ib/mad: create helper function for
>    [RFC PATCH 14/16] ib/mad: Create helper function for SMI processing
>    [RFC PATCH 08/16] ib/mad: Add Intel Omni-Path Architecture defines
>    [RFC PATCH 09/16] ib/mad: Implement support for Intel Omni-Path
>    [RFC PATCH 10/16] ib/mad: Add registration check for Intel Omni-Path
>    [RFC PATCH 15/16] ib/mad: Implement Intel Omni-Path Architecture SMP
>    [RFC PATCH 16/16] ib/mad: Implement Intel Omni-Path Architecture General
> 

Done.

> Another easy-to-fix nitpicking comment would be to have all the patches be
> consistent w.r.t to the capitalization of the 1st letter in the 1st word after the
> IB/core: or IB/mad:  prefix, e.g
> 
> ib/mad: create helper function for smi_handle_dr_smp_send
> 
> becomes
> 
> IB/mad: Create helper function for smi_handle_dr_smp_send

Done.

> 
> BTW, here my personal preference is "Add helper" and not "Create helper"
> 
> IB/mad: Add helper function for smi_handle_dr_smp_send

Done.

Ira

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš