On Mon, Oct 17, 2016 at 04:19:19AM -0700, Dennis Dalessandro wrote:
> @@ -183,6 +183,7 @@ static int hfi1_file_open(struct inode *inode, struct file *fp)
> fd->rec_cpu_num = -1; /* no cpu affinity by default */
> fd->mm = current->mm;
> atomic_inc(&fd->mm->mm_count);
> + atomic_inc(&dd->user_refcount);
This doesn't look like enough locking on acquire.. What prevents the
driver from already having been unloaded at this point?
> +static void wait_for_clients(struct hfi1_devdata *dd)
> +{
> + /*
> + * Remove the device init value and wait for active
> + * users to finish if there are any.
> + */
> + if (atomic_sub_return(1, &dd->user_refcount))
> + wait_for_completion(&dd->user_comp);
> +}
Shouldn't this be a while loop to avoid races?
.. and shouldn't the inc 'latch off' once the removal process starts?
I think you need to study how kernfs_drain is implemented and
basically copy that approach, including the locking.
FWIW, it is much easier and probably long term better to arrange
things so the FD can remain open but just return ENODEV to all calls.
Eg the srcu based approach in uverbs.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
