linux-kernel - Re: [PATCH v19 11/20] s390/vfio-ap: prepare for dynamic update of guest's APCB on queue probe/remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 7 Jun 2022 14:05:44 +0200
From:   Halil Pasic <pasic@...ux.ibm.com>
To:     Tony Krowiak <akrowiak@...ux.ibm.com>
Cc:     jjherne@...ux.ibm.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        freude@...ux.ibm.com, borntraeger@...ibm.com, cohuck@...hat.com,
        mjrosato@...ux.ibm.com, alex.williamson@...hat.com,
        kwankhede@...dia.com, fiuczy@...ux.ibm.com,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: [PATCH v19 11/20] s390/vfio-ap: prepare for dynamic update of
 guest's APCB on queue probe/remove

On Tue, 31 May 2022 06:44:46 -0400
Tony Krowiak <akrowiak@...ux.ibm.com> wrote:

> > vfio_ap_mdev_get_update_locks_for_apqn is "crazy long".
> > How about:
> >   get_mdev_for_apqn()
> >
> > This function is static and the terms mdev and apqn are specific 
> > enough that I
> > don't think it needs to start with vfio_ap. And there is no need to 
> > state in
> > the function name that locks are acquired. That point will be obvious 
> > to anyone
> > reading the prologue or the code.  
> 
> The primary purpose of the function is to acquire the locks in the 
> proper order, so
> I think the name should state that purpose. It may be obvious to someone 
> reading
> the prologue or this function, but not so obvious in the context of the 
> calling function.

I agree with Tony. To me get_mdev_for_apqn() sounds like getting a
reference to a matrix_mdev object (and incrementing its refcount) or
something similar. BTW some more bike shedding: I prefer by_apqn instead
of for_apqn, because the set of locks we need to take is determined _by_
the apqn parameter, but it ain't semantically the set of locks we need
to perform an update operation on the apqn or on the queue associated
with the apqn. No strong opinion though -- I'm no native speaker and
prepositions are difficult for me.

Regards,
Halil