> -----Original Message----- > From: Jason Gunthorpe [mailto:[email protected]] > Sent: Wednesday, May 06, 2015 10:18 PM > To: Devesh Sharma > Cc: Chuck Lever; [email protected]; Linux NFS Mailing List > Subject: Re: [PATCH v1 02/14] xprtrdma: Warn when there are orphaned IB > objects > > On Wed, May 06, 2015 at 07:52:03PM +0530, Devesh Sharma wrote: > > >> Should we check for EBUSY explicitly? other then this is an error > > >> in vendor specific ib_dealloc_pd() > > > > > > Any error return means ib_dealloc_pd() has failed, right? Doesn’t > > > that mean the PD is still allocated, and could cause problems later? > > > > Yes, you are correct, I was thinking ib_dealloc_pd() has a refcount > > implemented in the core layer, thus if the PD is used by any resource, > > it will always fail with -EBUSY. > > .. and it will not be freed, which indicates a serious bug in the caller, > so the > caller should respond to the failure with a BUG_ON or WARN_ON.
Yes, that’s what this patch is doing. > > > .With emulex adapter it is possible to fail dealloc_pd with ENOMEM or > > EIO in cases where device f/w is not responding etc. this situation do > > not represent PD is actually in use. > > This is a really bad idea. If the pd was freed and from the consumer's > perspective everything is sane then it should return success. > > If the driver detects an internal failure, then it should move the driver > to a > failed state (whatever that means, but at a minimum it means the firmware > state and driver state must be resync'd), and still succeed the dealloc. Makes sense. > > There is absolutely nothing the caller can do about a driver level failure > here, > and it doesn't indicate a caller bug. > > Returning ENOMEM for dealloc is what we'd call an insane API. You can't > have > failable memory allocations in a dealloc path. I will supply a fix in ocrdma. Reviewed-by: Devesh Sharma <[email protected]> > > Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
