Quoting r. Or Gerlitz <[EMAIL PROTECTED]>:
> Subject: [PATCH] [RFC] use device max_map_per_fmr in fmr pool
>
> Roland,
>
> Remapping an FMR the maximum number of times the device allows should
> reduce the amortized cost of using fmrs to the minimum possible.
>
>
> I was thinking on a patch of this spirit (which is not complete i know
> since down the code there's a usage of IB_FMR_MAX_REMAPS) but i figured
> out that mthca does not implement filling device_attr.max_map_per_fmr.
> >From what i understand, struct mthca_limits should be enhanced to support
> it, something which i failed to do... i think its related to
> 2^ log2(some function of the MPT size) but i am not sure.
>
> I understand that this relates also to the thread which does an
> optimizations for sinai... anyway it would be good if for 2.6.17
> the max remaps attribute would be supported so we can consider enhance
> the fmr pool to use it.
>
> Or.
Does this actually help performance?
How about increasing IB_FMR_MAX_REMAPS from 32 to say 128 and checking?
> Index: fmr_pool.c
> ===================================================================
> --- fmr_pool.c (revision 5524)
> +++ fmr_pool.c (working copy)
> @@ -214,6 +214,7 @@ struct ib_fmr_pool *ib_create_fmr_pool(s
> {
> struct ib_device *device;
> struct ib_fmr_pool *pool;
> + struct ib_device_attr device_attr;
> int i;
> int ret;
>
> @@ -228,6 +229,12 @@ struct ib_fmr_pool *ib_create_fmr_pool(s
> return ERR_PTR(-ENOSYS);
> }
>
> + ret = ib_query_device(device, &device_attr);
> + if (ret) {
> + printk(KERN_WARNING "couldn't query device");
> + return ERR_PTR(ret);
> + }
> +
> pool = kmalloc(sizeof *pool, GFP_KERNEL);
> if (!pool) {
> printk(KERN_WARNING "couldn't allocate pool struct");
> @@ -279,7 +286,7 @@ struct ib_fmr_pool *ib_create_fmr_pool(s
> struct ib_pool_fmr *fmr;
> struct ib_fmr_attr attr = {
> .max_pages = params->max_pages_per_fmr,
> - .max_maps = IB_FMR_MAX_REMAPS,
> + .max_maps = device_attr.max_map_per_fmr,
> .page_shift = params->page_shift
> };
Allocating resources up to a maximum supported by a device is rarely optimal.
Its easy to imagine a device where FMR number of maps is a shared resource,
so allocating one FMR with max_map_per_fmr will not let you create any more
of these.
Something like
.max_maps = min(IB_FMR_MAX_REMAPS, device_attr.max_map_per_fmr)
Would make more sense in my opinion.
We could also change ib_alloc_fmr implementation in mthca to report the actual
max_maps supported by this fmr (e.g. device could round this up to the power of
2 or something).
--
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
