IB/hfi1: Fix an assign/ordering issue with shared context IDs

The current algorithm for generating sub-context IDs is FILO.  If the
contexts are not closed in that order, the uniqueness of the ID will be
compromised. I.e. logging the creation/deletion of context IDs with an
application that assigns and closes in a FIFO order reveals:

cache_id: assign: uctxt: 3    sub_ctxt: 0
cache_id: assign: uctxt: 3    sub_ctxt: 1
cache_id: assign: uctxt: 3    sub_ctxt: 2
cache_id: close:  uctxt: 3    sub_ctxt: 0
cache_id: assign: uctxt: 3    sub_ctxt: 2 <<<

The sub_ctxt ID 2 is reused incorrectly.

Update the sub-context ID assign algorithm to use a bitmask of in_use
contexts.  The new algorithm will allow the contexts to be closed in any
order, and will only re-use unused contexts.

Size subctxt and subctxt_cnt to match the user API size.

Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
This commit is contained in:
Michael J. Ruhl 2017-05-04 05:15:15 -07:00 committed by Doug Ledford
parent 9b60d2cbe0
commit 8737ce95c4
6 changed files with 41 additions and 28 deletions

View File

@ -1289,7 +1289,7 @@ int hfi1_reset_device(int unit)
if (dd->rcd) if (dd->rcd)
for (i = dd->first_dyn_alloc_ctxt; for (i = dd->first_dyn_alloc_ctxt;
i < dd->num_rcv_contexts; i++) { i < dd->num_rcv_contexts; i++) {
if (!dd->rcd[i] || !dd->rcd[i]->cnt) if (!dd->rcd[i])
continue; continue;
spin_unlock_irqrestore(&dd->uctxt_lock, flags); spin_unlock_irqrestore(&dd->uctxt_lock, flags);
ret = -EBUSY; ret = -EBUSY;

View File

@ -49,6 +49,7 @@
#include <linux/vmalloc.h> #include <linux/vmalloc.h>
#include <linux/io.h> #include <linux/io.h>
#include <linux/sched/mm.h> #include <linux/sched/mm.h>
#include <linux/bitmap.h>
#include <rdma/ib.h> #include <rdma/ib.h>
@ -95,11 +96,10 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
struct hfi1_user_info *uinfo); struct hfi1_user_info *uinfo);
static unsigned int poll_urgent(struct file *fp, struct poll_table_struct *pt); static unsigned int poll_urgent(struct file *fp, struct poll_table_struct *pt);
static unsigned int poll_next(struct file *fp, struct poll_table_struct *pt); static unsigned int poll_next(struct file *fp, struct poll_table_struct *pt);
static int user_event_ack(struct hfi1_ctxtdata *uctxt, int subctxt, static int user_event_ack(struct hfi1_ctxtdata *uctxt, u16 subctxt,
unsigned long events); unsigned long events);
static int set_ctxt_pkey(struct hfi1_ctxtdata *uctxt, unsigned subctxt, static int set_ctxt_pkey(struct hfi1_ctxtdata *uctxt, u16 subctxt, u16 pkey);
u16 pkey); static int manage_rcvq(struct hfi1_ctxtdata *uctxt, u16 subctxt,
static int manage_rcvq(struct hfi1_ctxtdata *uctxt, unsigned subctxt,
int start_stop); int start_stop);
static int vma_fault(struct vm_fault *vmf); static int vma_fault(struct vm_fault *vmf);
static long hfi1_file_ioctl(struct file *fp, unsigned int cmd, static long hfi1_file_ioctl(struct file *fp, unsigned int cmd,
@ -773,8 +773,8 @@ static int hfi1_file_close(struct inode *inode, struct file *fp)
HFI1_MAX_SHARED_CTXTS) + fdata->subctxt; HFI1_MAX_SHARED_CTXTS) + fdata->subctxt;
*ev = 0; *ev = 0;
if (--uctxt->cnt) { __clear_bit(fdata->subctxt, uctxt->in_use_ctxts);
uctxt->active_slaves &= ~(1 << fdata->subctxt); if (!bitmap_empty(uctxt->in_use_ctxts, HFI1_MAX_SHARED_CTXTS)) {
mutex_unlock(&hfi1_mutex); mutex_unlock(&hfi1_mutex);
goto done; goto done;
} }
@ -868,7 +868,7 @@ static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo)
} }
/* /*
* Allocate a base context f context sharing is not required or we * Allocate a base context if context sharing is not required or we
* couldn't find a sub context. * couldn't find a sub context.
*/ */
if (!ret) if (!ret)
@ -905,17 +905,24 @@ static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo)
return ret; return ret;
} }
/*
* The hfi1_mutex must be held when this function is called. It is
* necessary to ensure serialized access to the bitmask in_use_ctxts.
*/
static int find_sub_ctxt(struct hfi1_filedata *fd, static int find_sub_ctxt(struct hfi1_filedata *fd,
const struct hfi1_user_info *uinfo) const struct hfi1_user_info *uinfo)
{ {
int i; int i;
struct hfi1_devdata *dd = fd->dd; struct hfi1_devdata *dd = fd->dd;
u16 subctxt;
for (i = dd->first_dyn_alloc_ctxt; i < dd->num_rcv_contexts; i++) { for (i = dd->first_dyn_alloc_ctxt; i < dd->num_rcv_contexts; i++) {
struct hfi1_ctxtdata *uctxt = dd->rcd[i]; struct hfi1_ctxtdata *uctxt = dd->rcd[i];
/* Skip ctxts which are not yet open */ /* Skip ctxts which are not yet open */
if (!uctxt || !uctxt->cnt) if (!uctxt ||
bitmap_empty(uctxt->in_use_ctxts,
HFI1_MAX_SHARED_CTXTS))
continue; continue;
/* Skip dynamically allocted kernel contexts */ /* Skip dynamically allocted kernel contexts */
@ -931,13 +938,19 @@ static int find_sub_ctxt(struct hfi1_filedata *fd,
continue; continue;
/* Verify the sharing process matches the master */ /* Verify the sharing process matches the master */
if (uctxt->userversion != uinfo->userversion || if (uctxt->userversion != uinfo->userversion)
uctxt->cnt >= uctxt->subctxt_cnt) {
return -EINVAL; return -EINVAL;
}
/* Find an unused context */
subctxt = find_first_zero_bit(uctxt->in_use_ctxts,
HFI1_MAX_SHARED_CTXTS);
if (subctxt >= uctxt->subctxt_cnt)
return -EINVAL;
fd->uctxt = uctxt; fd->uctxt = uctxt;
fd->subctxt = uctxt->cnt++; fd->subctxt = subctxt;
uctxt->active_slaves |= 1 << fd->subctxt; __set_bit(fd->subctxt, uctxt->in_use_ctxts);
return 1; return 1;
} }
@ -1055,7 +1068,7 @@ ctxdata_free:
static int init_subctxts(struct hfi1_ctxtdata *uctxt, static int init_subctxts(struct hfi1_ctxtdata *uctxt,
const struct hfi1_user_info *uinfo) const struct hfi1_user_info *uinfo)
{ {
unsigned num_subctxts; u16 num_subctxts;
num_subctxts = uinfo->subctxt_cnt; num_subctxts = uinfo->subctxt_cnt;
if (num_subctxts > HFI1_MAX_SHARED_CTXTS) if (num_subctxts > HFI1_MAX_SHARED_CTXTS)
@ -1063,7 +1076,6 @@ static int init_subctxts(struct hfi1_ctxtdata *uctxt,
uctxt->subctxt_cnt = uinfo->subctxt_cnt; uctxt->subctxt_cnt = uinfo->subctxt_cnt;
uctxt->subctxt_id = uinfo->subctxt_id; uctxt->subctxt_id = uinfo->subctxt_id;
uctxt->active_slaves = 1;
uctxt->redirect_seq_cnt = 1; uctxt->redirect_seq_cnt = 1;
set_bit(HFI1_CTXT_BASE_UNINIT, &uctxt->event_flags); set_bit(HFI1_CTXT_BASE_UNINIT, &uctxt->event_flags);
@ -1073,7 +1085,7 @@ static int init_subctxts(struct hfi1_ctxtdata *uctxt,
static int setup_subctxt(struct hfi1_ctxtdata *uctxt) static int setup_subctxt(struct hfi1_ctxtdata *uctxt)
{ {
int ret = 0; int ret = 0;
unsigned num_subctxts = uctxt->subctxt_cnt; u16 num_subctxts = uctxt->subctxt_cnt;
uctxt->subctxt_uregbase = vmalloc_user(PAGE_SIZE); uctxt->subctxt_uregbase = vmalloc_user(PAGE_SIZE);
if (!uctxt->subctxt_uregbase) if (!uctxt->subctxt_uregbase)
@ -1425,7 +1437,7 @@ done:
* overflow conditions. start_stop==1 re-enables, to be used to * overflow conditions. start_stop==1 re-enables, to be used to
* re-init the software copy of the head register * re-init the software copy of the head register
*/ */
static int manage_rcvq(struct hfi1_ctxtdata *uctxt, unsigned subctxt, static int manage_rcvq(struct hfi1_ctxtdata *uctxt, u16 subctxt,
int start_stop) int start_stop)
{ {
struct hfi1_devdata *dd = uctxt->dd; struct hfi1_devdata *dd = uctxt->dd;
@ -1460,7 +1472,7 @@ bail:
* User process then performs actions appropriate to bit having been * User process then performs actions appropriate to bit having been
* set, if desired, and checks again in future. * set, if desired, and checks again in future.
*/ */
static int user_event_ack(struct hfi1_ctxtdata *uctxt, int subctxt, static int user_event_ack(struct hfi1_ctxtdata *uctxt, u16 subctxt,
unsigned long events) unsigned long events)
{ {
int i; int i;
@ -1481,8 +1493,7 @@ static int user_event_ack(struct hfi1_ctxtdata *uctxt, int subctxt,
return 0; return 0;
} }
static int set_ctxt_pkey(struct hfi1_ctxtdata *uctxt, unsigned subctxt, static int set_ctxt_pkey(struct hfi1_ctxtdata *uctxt, u16 subctxt, u16 pkey)
u16 pkey)
{ {
int ret = -ENOENT, i, intable = 0; int ret = -ENOENT, i, intable = 0;
struct hfi1_pportdata *ppd = uctxt->ppd; struct hfi1_pportdata *ppd = uctxt->ppd;

View File

@ -228,7 +228,7 @@ struct hfi1_ctxtdata {
unsigned ctxt; unsigned ctxt;
/* /*
* non-zero if ctxt can be shared, and defines the maximum number of * non-zero if ctxt can be shared, and defines the maximum number of
* sub contexts allowed. * sub-contexts for this device context.
*/ */
u16 subctxt_cnt; u16 subctxt_cnt;
/* non-zero if ctxt is being shared. */ /* non-zero if ctxt is being shared. */
@ -287,10 +287,10 @@ struct hfi1_ctxtdata {
void *subctxt_rcvegrbuf; void *subctxt_rcvegrbuf;
/* An array of pages for the eager header queue entries * N */ /* An array of pages for the eager header queue entries * N */
void *subctxt_rcvhdr_base; void *subctxt_rcvhdr_base;
/* Bitmask of in use context(s) */
DECLARE_BITMAP(in_use_ctxts, HFI1_MAX_SHARED_CTXTS);
/* The version of the library which opened this ctxt */ /* The version of the library which opened this ctxt */
u32 userversion; u32 userversion;
/* Bitmask of active slaves */
u32 active_slaves;
/* Type of packets or conditions we want to poll for */ /* Type of packets or conditions we want to poll for */
u16 poll_type; u16 poll_type;
/* receive packet sequence counter */ /* receive packet sequence counter */
@ -1239,9 +1239,9 @@ struct mmu_rb_handler;
struct hfi1_filedata { struct hfi1_filedata {
struct hfi1_devdata *dd; struct hfi1_devdata *dd;
struct hfi1_ctxtdata *uctxt; struct hfi1_ctxtdata *uctxt;
unsigned subctxt;
struct hfi1_user_sdma_comp_q *cq; struct hfi1_user_sdma_comp_q *cq;
struct hfi1_user_sdma_pkt_q *pq; struct hfi1_user_sdma_pkt_q *pq;
u16 subctxt;
/* for cpu affinity; -1 if none */ /* for cpu affinity; -1 if none */
int rec_cpu_num; int rec_cpu_num;
u32 tid_n_pinned; u32 tid_n_pinned;

View File

@ -53,6 +53,7 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/printk.h> #include <linux/printk.h>
#include <linux/hrtimer.h> #include <linux/hrtimer.h>
#include <linux/bitmap.h>
#include <rdma/rdma_vt.h> #include <rdma/rdma_vt.h>
#include "hfi.h" #include "hfi.h"
@ -222,7 +223,7 @@ struct hfi1_ctxtdata *hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, u32 ctxt,
INIT_LIST_HEAD(&rcd->qp_wait_list); INIT_LIST_HEAD(&rcd->qp_wait_list);
rcd->ppd = ppd; rcd->ppd = ppd;
rcd->dd = dd; rcd->dd = dd;
rcd->cnt = 1; __set_bit(0, rcd->in_use_ctxts);
rcd->ctxt = ctxt; rcd->ctxt = ctxt;
dd->rcd[ctxt] = rcd; dd->rcd[ctxt] = rcd;
rcd->numa_id = numa; rcd->numa_id = numa;

View File

@ -47,6 +47,7 @@
#include <linux/pci.h> #include <linux/pci.h>
#include <linux/delay.h> #include <linux/delay.h>
#include <linux/bitmap.h>
#include "hfi.h" #include "hfi.h"
#include "common.h" #include "common.h"
@ -189,7 +190,7 @@ void handle_user_interrupt(struct hfi1_ctxtdata *rcd)
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&dd->uctxt_lock, flags); spin_lock_irqsave(&dd->uctxt_lock, flags);
if (!rcd->cnt) if (bitmap_empty(rcd->in_use_ctxts, HFI1_MAX_SHARED_CTXTS))
goto done; goto done;
if (test_and_clear_bit(HFI1_CTXT_WAITING_RCV, &rcd->event_flags)) { if (test_and_clear_bit(HFI1_CTXT_WAITING_RCV, &rcd->event_flags)) {

View File

@ -58,7 +58,7 @@ extern uint extended_psn;
struct hfi1_user_sdma_pkt_q { struct hfi1_user_sdma_pkt_q {
struct list_head list; struct list_head list;
unsigned ctxt; unsigned ctxt;
unsigned subctxt; u16 subctxt;
u16 n_max_reqs; u16 n_max_reqs;
atomic_t n_reqs; atomic_t n_reqs;
u16 reqidx; u16 reqidx;