[openib-general] Re: [PATCHv2 1/2] resend: mthca support for max_map_per_fmr device attribute

[Or Gerlitz]

Roland Dreier wrote:
> I had a chance to look at this, and I don't believe it is precisely
> correct for mem-free HCAs with the current FMR implementation.
> 
>  > +	/* on memfull HCA an FMR can be remapped 2^B - 1 times where B < 32 is
>  > +	 * the number of bits which are not used for MPT addressing, on memfree
>  > +	 * HCA B=8 so an FMR can be remapped 255 times.
>  > +	 */
>  > +	if(!mthca_is_memfree(mdev))
>  > +		props->max_map_per_fmr = (1 << (32 -
>  > +					long_log2(mdev->limits.num_mpts))) - 1;
>  > +	else
>  > +		props->max_map_per_fmr = (1 << 8) - 1;
> 
> Look at mthca_arbel_map_phys_fmr().  The question is how often key
> will repeat after being indexed, and when MTHCA_FLAG_SINAI_OPT is not
> set, then the same increment is used in the mem-free case as in the
> Tavor case.
> 
> So I think the code I quoted should really be:
> 
> 	if (dev->mthca_flags & MTHCA_FLAG_SINAI_OPT)
> 		props->max_map_per_fmr = (1 << (32 -
> 					long_log2(mdev->limits.num_mpts))) - 1;
> 	else
> 		props->max_map_per_fmr = (1 << 8) - 1;
> 
> Do you agree?  If so I can fix this patch up myself and apply it.

Yes it makes sense, but you need the check should be

	if (!(dev->mthca_flags & MTHCA_FLAG_SINAI_OPT))

instead of

	if (dev->mthca_flags & MTHCA_FLAG_SINAI_OPT)

also, what about the other patch which changes fmr_pool.c to query the 
device, have you got(reviewed/accepted) it? i have modified it to 
allocate the device attr struct on the heap as you have asked.

Or.