Re: [PATCH v7 04/15] s390/vfio-ap: implement in-use callback for vfio_ap driver

[Tony Krowiak]

On 4/23/20 11:13 PM, Halil Pasic wrote:
> On Thu, 16 Apr 2020 10:45:20 -0400
> Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
>
> > On 4/16/20 7:18 AM, Cornelia Huck wrote:
> > > On Tue,  7 Apr 2020 15:20:04 -0400
> > > Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
> > >
> > > > Let's implement the callback to indicate when an APQN
> > > > is in use by the vfio_ap device driver. The callback is
> > > > invoked whenever a change to the apmask or aqmask would
> > > > result in one or more queue devices being removed from the driver. The
> > > > vfio_ap device driver will indicate a resource is in use
> > > > if the APQN of any of the queue devices to be removed are assigned to
> > > > any of the matrix mdevs under the driver's control.
> > > >
> > > > Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
> > > > ---
> > > >    drivers/s390/crypto/vfio_ap_drv.c     |  1 +
> > > >    drivers/s390/crypto/vfio_ap_ops.c     | 47 +++++++++++++++++----------
> > > >    drivers/s390/crypto/vfio_ap_private.h |  2 ++
> > > >    3 files changed, 33 insertions(+), 17 deletions(-)
> > > > @@ -1369,3 +1371,14 @@ void vfio_ap_mdev_remove_queue(struct ap_queue *queue)
> > > >    	kfree(q);
> > > >    	mutex_unlock(&matrix_dev->lock);
> > > >    }
> > > > +
> > > > +bool vfio_ap_mdev_resource_in_use(unsigned long *apm, unsigned long *aqm)
> > > > +{
> > > > +	bool in_use;
> > > > +
> > > > +	mutex_lock(&matrix_dev->lock);
> > > > +	in_use = vfio_ap_mdev_verify_no_sharing(NULL, apm, aqm) ? true : false;
> > > Maybe
> > >
> > > in_use = !!vfio_ap_mdev_verify_no_sharing(NULL, apm, aqm);
> > >
> > > ?
> > To be honest, I find the !! expression very confusing. Every time I see
> > it, I have
> > to spend time thinking about what the result of !! is going to be. I think
> > the statement should be left as-is because it more clearly expresses
> > the intent.
> This is discussion is just about cosmetics, I believe. Just a piece of
> advice: try to be sensitive about the community. In this community, and
> I believe in C general !! is the idiomatic way to convert number to
> boolean. Why would one want to do that is a bit longer story. The short
> version is in logic condition context the value 0 is false and any
> other value is true. !! keeps false value (0) false, and forces a true to
> the most true true value. If you keep getting confused every time you
> run across a !! that won't help with reading other peoples C.
>
> Regards,
> Halil

The point is moot. After seeing that Conny's comment generated a
discussion, I decided to avoid wasting additional time discussing
personal preferences and am now using the !! syntax. Unfortunately,
I've been having some odd problems with my email client and my
response to Pierre's comment never made it to the list, so I apologize
that you had to waste valuable time on your tutorial.

> > > > +	mutex_unlock(&matrix_dev->lock);
> > > > +
> > > > +	return in_use;
> > > > +}