Re: [PATCH for-next 8/9] RDMA/hns: Kernel notify usr space to stop ring db

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Aug 12, 2019 at 08:52:20AM +0300, Leon Romanovsky wrote:
> On Fri, Aug 09, 2019 at 05:41:05PM +0800, Lijun Ou wrote:
> > From: Yangyang Li <liyangyang20@huawei.com>
> >
> > In the reset scenario, if the kernel receives the reset signal,
> > it needs to notify the user space to stop ring doorbell.
> 
> I doubt about it, it is racy like hell and relies on assumption that
> userspace will honor such request to stop.

Sounds like this is the device unplug flow we already have support
for, use the APIs to drop the VMA refering to the doorbell

> > Signed-off-by: Yangyang Li <liyangyang20@huawei.com>
> >  drivers/infiniband/hw/hns/hns_roce_device.h |  4 +++
> >  drivers/infiniband/hw/hns/hns_roce_hw_v2.c  | 52 ++++++++++++++++++++++++++++-
> >  drivers/infiniband/hw/hns/hns_roce_hw_v2.h  |  4 +++
> >  drivers/infiniband/hw/hns/hns_roce_main.c   | 22 ++++++------
> >  4 files changed, 70 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
> > index 32465f5..be65fce 100644
> > +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> > @@ -268,6 +268,8 @@ enum {
> >
> >  #define PAGE_ADDR_SHIFT				12
> >
> > +#define HNS_ROCE_IS_RESETTING			1
> > +
> >  struct hns_roce_uar {
> >  	u64		pfn;
> >  	unsigned long	index;
> > @@ -1043,6 +1045,8 @@ struct hns_roce_dev {
> >  	u32			odb_offset;
> >  	dma_addr_t		tptr_dma_addr;	/* only for hw v1 */
> >  	u32			tptr_size;	/* only for hw v1 */
> > +	struct page		*reset_page; /* store reset state */
> > +	void			*reset_kaddr; /* addr of reset page */
> >  	const struct hns_roce_hw *hw;
> >  	void			*priv;
> >  	struct workqueue_struct *irq_workq;
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> > index d33341e..138e5a8 100644
> > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> > @@ -1867,17 +1867,49 @@ static void hns_roce_free_link_table(struct hns_roce_dev *hr_dev,
> >  			  link_tbl->table.map);
> >  }
> >
> > +static int hns_roce_v2_get_reset_page(struct hns_roce_dev *hr_dev)
> > +{
> > +	hr_dev->reset_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
> > +	if (!hr_dev->reset_page)
> > +		return -ENOMEM;
> > +
> > +	hr_dev->reset_kaddr = vmap(&hr_dev->reset_page, 1, VM_MAP, PAGE_KERNEL);
> > +	if (!hr_dev->reset_kaddr)
> > +		goto err_with_vmap;

Yes, this vmap is nonsense too, get_zeroed_page() is the right API

Jason