[lustre-devel] [PATCH 01/10] lustre: lnd: set device capabilities

From: NeilBrown <neilb@suse.com>
To: lustre-devel@lists.lustre.org
Subject: [lustre-devel] [PATCH 01/10] lustre: lnd: set device capabilities
Date: Mon, 22 Oct 2018 13:48:19 +1100	[thread overview]
Message-ID: <87va5u8znw.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <alpine.LFD.2.21.1810201748160.16676@casper.infradead.org>

On Sat, Oct 20 2018, James Simmons wrote:

>> On Sun, Oct 14 2018, James Simmons wrote:
>> 
>> > From: Amir Shehata <ashehata@whamcloud.com>
>> >
>> > MLX-4, MLX-5 and OPA support different capabilities. Query the
>> > device and cache the capabilities of the device for future use.
>> >
>> > At the time of the patches creation MLX5 could support fast
>> > registration and gaps while MLX4 and OPA only support FMR
>> >
>> > Signed-off-by: Amir Shehata <ashehata@whamcloud.com>
>> > WC-bug-id: https://jira.whamcloud.com/browse/LU-10129
>> > Reviewed-on: https://review.whamcloud.com/30309
>> > Reviewed-by: Alexey Lyashkov <c17817@cray.com>
>> > Reviewed-by: Dmitry Eremin <dmitry.eremin@intel.com>
>> > Reviewed-by: James Simmons <uja.ornl@yahoo.com>
>> > Reviewed-by: Oleg Drokin <green@whamcloud.com>
>> > Signed-off-by: James Simmons <jsimmons@infradead.org>
>> > ---
>> >  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    | 41 ++++++++++++----------
>> >  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h    |  5 +++
>> >  2 files changed, 28 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> > index bf969b3..b10658b 100644
>> > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> > @@ -1399,6 +1399,7 @@ static int kiblnd_alloc_fmr_pool(struct kib_fmr_poolset *fps, struct kib_fmr_poo
>> >  		else
>> >  			CERROR("FMRs are not supported\n");
>> >  	}
>> > +	fpo->fpo_is_fmr = true;
>> >  
>> >  	return rc;
>> >  }
>> > @@ -1408,6 +1409,8 @@ static int kiblnd_alloc_freg_pool(struct kib_fmr_poolset *fps, struct kib_fmr_po
>> >  	struct kib_fast_reg_descriptor *frd;
>> >  	int i, rc;
>> >  
>> > +	fpo->fpo_is_fmr = false;
>> > +
>> >  	INIT_LIST_HEAD(&fpo->fast_reg.fpo_pool_list);
>> >  	fpo->fast_reg.fpo_pool_size = 0;
>> >  	for (i = 0; i < fps->fps_pool_size; i++) {
>> > @@ -1469,23 +1472,7 @@ static int kiblnd_create_fmr_pool(struct kib_fmr_poolset *fps,
>> >  	fpo->fpo_hdev = kiblnd_current_hdev(dev);
>> >  	dev_attr = &fpo->fpo_hdev->ibh_ibdev->attrs;
>> >  
>> > -	/* Check for FMR or FastReg support */
>> > -	fpo->fpo_is_fmr = 0;
>> > -	if (fpo->fpo_hdev->ibh_ibdev->alloc_fmr &&
>> > -	    fpo->fpo_hdev->ibh_ibdev->dealloc_fmr &&
>> > -	    fpo->fpo_hdev->ibh_ibdev->map_phys_fmr &&
>> > -	    fpo->fpo_hdev->ibh_ibdev->unmap_fmr) {
>> > -		LCONSOLE_INFO("Using FMR for registration\n");
>> > -		fpo->fpo_is_fmr = 1;
>> > -	} else if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
>> > -		LCONSOLE_INFO("Using FastReg for registration\n");
>> > -	} else {
>> > -		rc = -ENOSYS;
>> > -		LCONSOLE_ERROR_MSG(rc, "IB device does not support FMRs nor FastRegs, can't register memory\n");
>> > -		goto out_fpo;
>> > -	}
>> > -
>> > -	if (fpo->fpo_is_fmr)
>> > +	if (dev->ibd_dev_caps & IBLND_DEV_CAPS_FMR_ENABLED)
>> >  		rc = kiblnd_alloc_fmr_pool(fps, fpo);
>> >  	else
>> >  		rc = kiblnd_alloc_freg_pool(fps, fpo);
>> > @@ -2261,6 +2248,9 @@ static int kiblnd_net_init_pools(struct kib_net *net, struct lnet_ni *ni,
>> >  
>> >  static int kiblnd_hdev_get_attr(struct kib_hca_dev *hdev)
>> >  {
>> > +	struct ib_device_attr *dev_attr = &hdev->ibh_ibdev->attrs;
>> > +	int rc = 0;
>> > +
>> >  	/*
>> >  	 * It's safe to assume a HCA can handle a page size
>> >  	 * matching that of the native system
>> > @@ -2269,7 +2259,22 @@ static int kiblnd_hdev_get_attr(struct kib_hca_dev *hdev)
>> >  	hdev->ibh_page_size  = 1 << PAGE_SHIFT;
>> >  	hdev->ibh_page_mask  = ~((__u64)hdev->ibh_page_size - 1);
>> >  
>> > -	hdev->ibh_mr_size = hdev->ibh_ibdev->attrs.max_mr_size;
>> > +	if (hdev->ibh_ibdev->alloc_fmr &&
>> > +	    hdev->ibh_ibdev->dealloc_fmr &&
>> > +	    hdev->ibh_ibdev->map_phys_fmr &&
>> > +	    hdev->ibh_ibdev->unmap_fmr) {
>> > +		LCONSOLE_INFO("Using FMR for registration\n");
>> > +		hdev->ibh_dev->ibd_dev_caps |= IBLND_DEV_CAPS_FMR_ENABLED;
>> > +	} else if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
>> > +		LCONSOLE_INFO("Using FastReg for registration\n");
>> > +		hdev->ibh_dev->ibd_dev_caps |= IBLND_DEV_CAPS_FASTREG_ENABLED;
>> > +	} else {
>> > +		CERROR("IB device does not support FMRs nor FastRegs, can't register memory: %d\n",
>> > +		       rc);
>> > +		return -ENXIO;
>> > +	}
>> > +
>> > +	hdev->ibh_mr_size = dev_attr->max_mr_size;
>> >  	if (hdev->ibh_mr_size == ~0ULL) {
>> >  		hdev->ibh_mr_shift = 64;
>> >  		return 0;
>> > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
>> > index a4438d2..9f0a47d 100644
>> > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
>> > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
>> > @@ -73,6 +73,10 @@
>> >  #define IBLND_N_SCHED			2
>> >  #define IBLND_N_SCHED_HIGH		4
>> >  
>> > +#define IBLND_DEV_CAPS_FASTREG_ENABLED		0x1
>> > +#define IBLND_DEV_CAPS_FASTREG_GAPS_SUPPORT	0x2
>> > +#define IBLND_DEV_CAPS_FMR_ENABLED		0x4
>> > +
>> 
>> BIT(0), BIT(1), .... ???
>> 
>> >  struct kib_tunables {
>> >  	int *kib_dev_failover;           /* HCA failover */
>> >  	unsigned int *kib_service;       /* IB service number */
>> > @@ -162,6 +166,7 @@ struct kib_dev {
>> >  	unsigned int ibd_can_failover; /* IPoIB interface is a bonding master */
>> >  	struct list_head   ibd_nets;
>> >  	struct kib_hca_dev *ibd_hdev;
>> > +	u32			ibd_dev_caps;
>> 
>> "unsigned int" would be better I think, but it isn't very important.
>
> Actually better yet it could be changed to an enum. Move the below to
> just above struct kib_dev {
>
> enum ibd_dev_caps {
> 	IBLND_DEV_CAPS_FASTREG_ENABLED		= BIT(0),
> 	IBLND_DEV_CAPS_FASTREG_GAPS_SUPPORT	= BIT(1),
> 	IBLND_DEV_CAPS_FMR_ENABLED		= BIT(2),
> }
>
> and change ibd_dev_caps from u32 to enum ibd_dev_caps.
>
> Do you mind if that is a follow on patch?
>

Yes, that would be good, thanks.
I'll apply this patch as it is.

Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20181022/921e6766/attachment.sig>