Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

IB/core: Make ib_dealloc_pd return void

The majority of callers never check the return value, and even if they
did, they can't do anything about a failure.

All possible failure cases represent a bug in the caller, so just
WARN_ON inside the function instead.

This fixes a few random errors:
net/rd/iw.c infinite loops while it fails. (racing with EBUSY?)

This also lays the ground work to get rid of error return from the
drivers. Most drivers do not error, the few that do are broken since
it cannot be handled.

Since uverbs can legitimately make use of EBUSY, open code the
check.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>

authored by

Jason Gunthorpe and committed by
Doug Ledford
7dd78647 03f6fb93

+41 -26
+16 -6
drivers/infiniband/core/uverbs_cmd.c
··· 606 606 { 607 607 struct ib_uverbs_dealloc_pd cmd; 608 608 struct ib_uobject *uobj; 609 + struct ib_pd *pd; 609 610 int ret; 610 611 611 612 if (copy_from_user(&cmd, buf, sizeof cmd)) ··· 615 614 uobj = idr_write_uobj(&ib_uverbs_pd_idr, cmd.pd_handle, file->ucontext); 616 615 if (!uobj) 617 616 return -EINVAL; 617 + pd = uobj->object; 618 618 619 - ret = ib_dealloc_pd(uobj->object); 620 - if (!ret) 621 - uobj->live = 0; 619 + if (atomic_read(&pd->usecnt)) { 620 + ret = -EBUSY; 621 + goto err_put; 622 + } 622 623 623 - put_uobj_write(uobj); 624 - 624 + ret = pd->device->dealloc_pd(uobj->object); 625 + WARN_ONCE(ret, "Infiniband HW driver failed dealloc_pd"); 625 626 if (ret) 626 - return ret; 627 + goto err_put; 628 + 629 + uobj->live = 0; 630 + put_uobj_write(uobj); 627 631 628 632 idr_remove_uobj(&ib_uverbs_pd_idr, uobj); 629 633 ··· 639 633 put_uobj(uobj); 640 634 641 635 return in_len; 636 + 637 + err_put: 638 + put_uobj_write(uobj); 639 + return ret; 642 640 } 643 641 644 642 struct xrcd_table_entry {
+20 -6
drivers/infiniband/core/verbs.c
··· 260 260 } 261 261 EXPORT_SYMBOL(ib_alloc_pd); 262 262 263 - int ib_dealloc_pd(struct ib_pd *pd) 263 + /** 264 + * ib_dealloc_pd - Deallocates a protection domain. 265 + * @pd: The protection domain to deallocate. 266 + * 267 + * It is an error to call this function while any resources in the pd still 268 + * exist. The caller is responsible to synchronously destroy them and 269 + * guarantee no new allocations will happen. 270 + */ 271 + void ib_dealloc_pd(struct ib_pd *pd) 264 272 { 273 + int ret; 274 + 265 275 if (pd->local_mr) { 266 - if (ib_dereg_mr(pd->local_mr)) 267 - return -EBUSY; 276 + ret = ib_dereg_mr(pd->local_mr); 277 + WARN_ON(ret); 268 278 pd->local_mr = NULL; 269 279 } 270 280 271 - if (atomic_read(&pd->usecnt)) 272 - return -EBUSY; 281 + /* uverbs manipulates usecnt with proper locking, while the kabi 282 + requires the caller to guarantee we can't race here. */ 283 + WARN_ON(atomic_read(&pd->usecnt)); 273 284 274 - return pd->device->dealloc_pd(pd); 285 + /* Making delalloc_pd a void return is a WIP, no driver should return 286 + an error here. */ 287 + ret = pd->device->dealloc_pd(pd); 288 + WARN_ONCE(ret, "Infiniband HW driver failed dealloc_pd"); 275 289 } 276 290 EXPORT_SYMBOL(ib_dealloc_pd); 277 291
+1 -3
drivers/infiniband/ulp/ipoib/ipoib_verbs.c
··· 280 280 priv->wq = NULL; 281 281 } 282 282 283 - if (ib_dealloc_pd(priv->pd)) 284 - ipoib_warn(priv, "ib_dealloc_pd failed\n"); 285 - 283 + ib_dealloc_pd(priv->pd); 286 284 } 287 285 288 286 void ipoib_event(struct ib_event_handler *handler,
+1 -1
drivers/infiniband/ulp/iser/iser_verbs.c
··· 185 185 186 186 (void)ib_unregister_event_handler(&device->event_handler); 187 187 (void)ib_dereg_mr(device->mr); 188 - (void)ib_dealloc_pd(device->pd); 188 + ib_dealloc_pd(device->pd); 189 189 190 190 kfree(device->comps); 191 191 device->comps = NULL;
+1 -5
include/rdma/ib_verbs.h
··· 2196 2196 2197 2197 struct ib_pd *ib_alloc_pd(struct ib_device *device); 2198 2198 2199 - /** 2200 - * ib_dealloc_pd - Deallocates a protection domain. 2201 - * @pd: The protection domain to deallocate. 2202 - */ 2203 - int ib_dealloc_pd(struct ib_pd *pd); 2199 + void ib_dealloc_pd(struct ib_pd *pd); 2204 2200 2205 2201 /** 2206 2202 * ib_create_ah - Creates an address handle for the given address vector.
+1 -4
net/rds/iw.c
··· 148 148 if (rds_iwdev->mr) 149 149 ib_dereg_mr(rds_iwdev->mr); 150 150 151 - while (ib_dealloc_pd(rds_iwdev->pd)) { 152 - rdsdebug("Failed to dealloc pd %p\n", rds_iwdev->pd); 153 - msleep(1); 154 - } 151 + ib_dealloc_pd(rds_iwdev->pd); 155 152 156 153 list_del(&rds_iwdev->list); 157 154 kfree(rds_iwdev);
+1 -1
net/sunrpc/xprtrdma/verbs.c
··· 624 624 625 625 /* If the pd is still busy, xprtrdma missed freeing a resource */ 626 626 if (ia->ri_pd && !IS_ERR(ia->ri_pd)) 627 - WARN_ON(ib_dealloc_pd(ia->ri_pd)); 627 + ib_dealloc_pd(ia->ri_pd); 628 628 } 629 629 630 630 /*