Re: [PATCH 7/8] vfio: Follow the naming pattern for vfio_group_ioctl_unset_container()

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Aug 30, 2022 at 10:42:31AM -0600, Alex Williamson wrote:
> On Mon, 29 Aug 2022 12:28:07 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Mon, Aug 29, 2022 at 12:41:30AM +0000, Tian, Kevin wrote:
> > > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > > Sent: Thursday, August 18, 2022 12:07 AM
> > > > 
> > > > Make it clear that this is the body of the ioctl - keep the mutex outside
> > > > the function since this function doesn't have and wouldn't benefit from
> > > > error unwind.  
> > > 
> > > but doing so make unset_container() unpair with set_container() and
> > > be the only one doing additional things in main ioctl body.
> > > 
> > > I'd prefer to moving mutex inside unset_container() for better readability.  
> > 
> > Yes, I tried both ways and ended up here since adding the goto unwind
> > was kind of ungainly for this function. Don't mind either way
> > 
> > The functions are not intended as strict pairs, they are ioctl
> > dispatch functions.
> 
> The lockdep annotation seems sufficient, but what about simply
> prefixing the unset ioctl function with underscores to reinforce the
> locking requirement, as done by the called function
> __vfio_group_unset_container()?  Thanks,

Could do, but IMHO, that is not a popular convention in VFIO
(anymore?).

I've been adding lockdep annotations, not adding __ to indicate
the function is called with some kind of lock.

In my tree __vfio_group_get_from_iommu() is the only one left using
that style. uset_container was turned into
vfio_container_detatch_group() in aother series

Conversly we have __vfio_register_dev() which I guess __ indicates is
internal or wrappered, not some statement about locking. I think this
has become the more popular usage of the __ generally in the kernel

So I will do a v2 with the extra goto unwind then

Are there any other remarks before I do it?

Thanks,
Jason