Skip to content

Commit

Permalink
IB/hfi1: Protect context array set/clear with spinlock
Browse files Browse the repository at this point in the history 
The rcd array can be accessed from user context or during interrupts.
Protecting this with a mutex isn't a good idea because the mutex should
not be used from an IRQ.

Protect the allocation and freeing of rcd array elements with a
spinlock.

Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Reviewed-by: Sebastian Sanchez <sebastian.sanchez@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>
  • Loading branch information
Michael J. Ruhl authored and Doug Ledford committed Aug 22, 2017
1 parent 64a296f commit f2a3bc0
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 181 deletions.
10 changes: 8 additions & 2 deletions drivers/infiniband/hw/hfi1/chip.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15054,10 +15054,16 @@ struct hfi1_devdata *hfi1_init_dd(struct pci_dev *pdev,
if (ret)
goto bail_cleanup;

ret = hfi1_create_ctxts(dd);
ret = hfi1_create_kctxts(dd);
if (ret)
goto bail_cleanup;

/*
* Initialize aspm, to be done after gen3 transition and setting up
* contexts and before enabling interrupts
*/
aspm_init(dd);

dd->rcvhdrsize = DEFAULT_RCVHDRSIZE;
/*
* rcd[0] is guaranteed to be valid by this point. Also, all
Expand All @@ -15076,7 +15082,7 @@ struct hfi1_devdata *hfi1_init_dd(struct pci_dev *pdev,
goto bail_cleanup;
}

/* use contexts created by hfi1_create_ctxts */
/* use contexts created by hfi1_create_kctxts */
ret = set_up_interrupts(dd);
if (ret)
goto bail_cleanup;
Expand Down
171 changes: 74 additions & 97 deletions drivers/infiniband/hw/hfi1/file_ops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma);

static u64 kvirt_to_phys(void *addr);
static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo);
static int init_subctxts(struct hfi1_ctxtdata *uctxt,
const struct hfi1_user_info *uinfo);
static void init_subctxts(struct hfi1_ctxtdata *uctxt,
const struct hfi1_user_info *uinfo);
static int init_user_ctxt(struct hfi1_filedata *fd,
struct hfi1_ctxtdata *uctxt);
static void user_init(struct hfi1_ctxtdata *uctxt);
Expand Down Expand Up @@ -758,7 +758,6 @@ static int hfi1_file_close(struct inode *inode, struct file *fp)
goto done;

hfi1_cdbg(PROC, "freeing ctxt %u:%u", uctxt->ctxt, fdata->subctxt);
mutex_lock(&hfi1_mutex);

flush_wc();
/* drain user sdma queue */
Expand All @@ -778,6 +777,7 @@ static int hfi1_file_close(struct inode *inode, struct file *fp)
HFI1_MAX_SHARED_CTXTS) + fdata->subctxt;
*ev = 0;

mutex_lock(&hfi1_mutex);
__clear_bit(fdata->subctxt, uctxt->in_use_ctxts);
fdata->uctxt = NULL;
hfi1_rcd_put(uctxt); /* fdata reference */
Expand Down Expand Up @@ -844,6 +844,38 @@ static u64 kvirt_to_phys(void *addr)
return paddr;
}

static int complete_subctxt(struct hfi1_filedata *fd)
{
int ret;

/*
* sub-context info can only be set up after the base context
* has been completed.
*/
ret = wait_event_interruptible(
fd->uctxt->wait,
!test_bit(HFI1_CTXT_BASE_UNINIT, &fd->uctxt->event_flags));

if (test_bit(HFI1_CTXT_BASE_FAILED, &fd->uctxt->event_flags))
ret = -ENOMEM;

/* The only thing a sub context needs is the user_xxx stuff */
if (!ret) {
fd->rec_cpu_num = hfi1_get_proc_affinity(fd->uctxt->numa_id);
ret = init_user_ctxt(fd, fd->uctxt);
}

if (ret) {
hfi1_rcd_put(fd->uctxt);
fd->uctxt = NULL;
mutex_lock(&hfi1_mutex);
__clear_bit(fd->subctxt, fd->uctxt->in_use_ctxts);
mutex_unlock(&hfi1_mutex);
}

return ret;
}

static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo)
{
int ret;
Expand All @@ -854,79 +886,57 @@ static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo)
if (swmajor != HFI1_USER_SWMAJOR)
return -ENODEV;

if (uinfo->subctxt_cnt > HFI1_MAX_SHARED_CTXTS)
return -EINVAL;

swminor = uinfo->userversion & 0xffff;

/*
* Acquire the mutex to protect against multiple creations of what
* could be a shared base context.
*/
mutex_lock(&hfi1_mutex);
/*
* Get a sub context if necessary.
* Get a sub context if available (fd->uctxt will be set).
* ret < 0 error, 0 no context, 1 sub-context found
*/
ret = 0;
if (uinfo->subctxt_cnt) {
ret = find_sub_ctxt(fd, uinfo);
if (ret > 0)
fd->rec_cpu_num =
hfi1_get_proc_affinity(fd->uctxt->numa_id);
}
ret = find_sub_ctxt(fd, uinfo);

/*
* Allocate a base context if context sharing is not required or we
* couldn't find a sub context.
* Allocate a base context if context sharing is not required or a
* sub context wasn't found.
*/
if (!ret)
ret = allocate_ctxt(fd, fd->dd, uinfo, &uctxt);

mutex_unlock(&hfi1_mutex);

/* Depending on the context type, do the appropriate init */
if (ret > 0) {
/*
* sub-context info can only be set up after the base
* context has been completed.
*/
ret = wait_event_interruptible(fd->uctxt->wait, !test_bit(
HFI1_CTXT_BASE_UNINIT,
&fd->uctxt->event_flags));
if (test_bit(HFI1_CTXT_BASE_FAILED, &fd->uctxt->event_flags))
ret = -ENOMEM;

/* The only thing a sub context needs is the user_xxx stuff */
if (!ret)
ret = init_user_ctxt(fd, fd->uctxt);

if (ret)
clear_bit(fd->subctxt, fd->uctxt->in_use_ctxts);

} else if (!ret) {
switch (ret) {
case 0:
ret = setup_base_ctxt(fd, uctxt);
if (uctxt->subctxt_cnt) {
/* If there is an error, set the failed bit. */
if (ret)
set_bit(HFI1_CTXT_BASE_FAILED,
&uctxt->event_flags);
/*
* Base context is done, notify anybody using a
* sub-context that is waiting for this completion
*/
clear_bit(HFI1_CTXT_BASE_UNINIT, &uctxt->event_flags);
wake_up(&uctxt->wait);
}
if (ret)
deallocate_ctxt(uctxt);
}

/* If an error occurred, clear the reference */
if (ret && fd->uctxt) {
hfi1_rcd_put(fd->uctxt);
fd->uctxt = NULL;
break;
case 1:
ret = complete_subctxt(fd);
break;
default:
break;
}

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.
* necessary to ensure serialized creation of shared contexts.
*/
static int find_sub_ctxt(struct hfi1_filedata *fd,
const struct hfi1_user_info *uinfo)
Expand All @@ -935,6 +945,9 @@ static int find_sub_ctxt(struct hfi1_filedata *fd,
struct hfi1_devdata *dd = fd->dd;
u16 subctxt;

if (!uinfo->subctxt_cnt)
return 0;

for (i = dd->first_dyn_alloc_ctxt; i < dd->num_rcv_contexts; i++) {
struct hfi1_ctxtdata *uctxt = dd->rcd[i];

Expand Down Expand Up @@ -983,7 +996,6 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
struct hfi1_ctxtdata **cd)
{
struct hfi1_ctxtdata *uctxt;
u16 ctxt;
int ret, numa;

if (dd->flags & HFI1_FROZEN) {
Expand All @@ -997,22 +1009,9 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
return -EIO;
}

/*
* This check is sort of redundant to the next EBUSY error. It would
* also indicate an inconsistancy in the driver if this value was
* zero, but there were still contexts available.
*/
if (!dd->freectxts)
return -EBUSY;

for (ctxt = dd->first_dyn_alloc_ctxt;
ctxt < dd->num_rcv_contexts; ctxt++)
if (!dd->rcd[ctxt])
break;

if (ctxt == dd->num_rcv_contexts)
return -EBUSY;

/*
* If we don't have a NUMA node requested, preference is towards
* device NUMA node.
Expand All @@ -1022,11 +1021,10 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
numa = cpu_to_node(fd->rec_cpu_num);
else
numa = numa_node_id();
uctxt = hfi1_create_ctxtdata(dd->pport, ctxt, numa);
if (!uctxt) {
dd_dev_err(dd,
"Unable to allocate ctxtdata memory, failing open\n");
return -ENOMEM;
ret = hfi1_create_ctxtdata(dd->pport, numa, &uctxt);
if (ret < 0) {
dd_dev_err(dd, "user ctxtdata allocation failed\n");
return ret;
}
hfi1_cdbg(PROC, "[%u:%u] pid %u assigned to CPU %d (NUMA %u)",
uctxt->ctxt, fd->subctxt, current->pid, fd->rec_cpu_num,
Expand All @@ -1035,8 +1033,7 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
/*
* Allocate and enable a PIO send context.
*/
uctxt->sc = sc_alloc(dd, SC_USER, uctxt->rcvhdrqentsize,
uctxt->dd->node);
uctxt->sc = sc_alloc(dd, SC_USER, uctxt->rcvhdrqentsize, dd->node);
if (!uctxt->sc) {
ret = -ENOMEM;
goto ctxdata_free;
Expand All @@ -1048,20 +1045,13 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
goto ctxdata_free;

/*
* Setup sub context resources if the user-level has requested
* Setup sub context information if the user-level has requested
* sub contexts.
* This has to be done here so the rest of the sub-contexts find the
* proper master.
* proper base context.
*/
if (uinfo->subctxt_cnt) {
ret = init_subctxts(uctxt, uinfo);
/*
* On error, we don't need to disable and de-allocate the
* send context because it will be done during file close
*/
if (ret)
goto ctxdata_free;
}
if (uinfo->subctxt_cnt)
init_subctxts(uctxt, uinfo);
uctxt->userversion = uinfo->userversion;
uctxt->flags = hfi1_cap_mask; /* save current flag state */
init_waitqueue_head(&uctxt->wait);
Expand All @@ -1081,9 +1071,7 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
return 0;

ctxdata_free:
*cd = NULL;
dd->rcd[ctxt] = NULL;
hfi1_rcd_put(uctxt);
hfi1_free_ctxt(dd, uctxt);
return ret;
}

Expand All @@ -1093,28 +1081,17 @@ static void deallocate_ctxt(struct hfi1_ctxtdata *uctxt)
hfi1_stats.sps_ctxts--;
if (++uctxt->dd->freectxts == uctxt->dd->num_user_contexts)
aspm_enable_all(uctxt->dd);

/* _rcd_put() should be done after releasing mutex */
uctxt->dd->rcd[uctxt->ctxt] = NULL;
mutex_unlock(&hfi1_mutex);
hfi1_rcd_put(uctxt); /* dd reference */

hfi1_free_ctxt(uctxt->dd, uctxt);
}

static int init_subctxts(struct hfi1_ctxtdata *uctxt,
const struct hfi1_user_info *uinfo)
static void init_subctxts(struct hfi1_ctxtdata *uctxt,
const struct hfi1_user_info *uinfo)
{
u16 num_subctxts;

num_subctxts = uinfo->subctxt_cnt;
if (num_subctxts > HFI1_MAX_SHARED_CTXTS)
return -EINVAL;

uctxt->subctxt_cnt = uinfo->subctxt_cnt;
uctxt->subctxt_id = uinfo->subctxt_id;
uctxt->redirect_seq_cnt = 1;
set_bit(HFI1_CTXT_BASE_UNINIT, &uctxt->event_flags);

return 0;
}

static int setup_subctxt(struct hfi1_ctxtdata *uctxt)
Expand Down Expand Up @@ -1302,8 +1279,8 @@ static int setup_base_ctxt(struct hfi1_filedata *fd,
return 0;

setup_failed:
/* Call _free_ctxtdata, not _rcd_put(). We still need the context. */
hfi1_free_ctxtdata(dd, uctxt);
set_bit(HFI1_CTXT_BASE_FAILED, &uctxt->event_flags);
deallocate_ctxt(uctxt);
return ret;
}

Expand Down
8 changes: 4 additions & 4 deletions drivers/infiniband/hw/hfi1/hfi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ struct hfi1_ctxtdata {
u16 poll_type;
/* receive packet sequence counter */
u8 seq_cnt;
u8 redirect_seq_cnt;
/* ctxt rcvhdrq head offset */
u32 head;
/* QPs waiting for context processing */
Expand Down Expand Up @@ -1263,9 +1262,10 @@ void handle_user_interrupt(struct hfi1_ctxtdata *rcd);

int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd);
int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd);
int hfi1_create_ctxts(struct hfi1_devdata *dd);
struct hfi1_ctxtdata *hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, u16 ctxt,
int numa);
int hfi1_create_kctxts(struct hfi1_devdata *dd);
int hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, int numa,
struct hfi1_ctxtdata **rcd);
void hfi1_free_ctxt(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd);
void hfi1_init_pportdata(struct pci_dev *pdev, struct hfi1_pportdata *ppd,
struct hfi1_devdata *dd, u8 hw_pidx, u8 port);
void hfi1_free_ctxtdata(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd);
Expand Down
Loading

0 comments on commit f2a3bc0

Please sign in to comment.