linux-kernel - Re: [PATCH v18 08/18] s390/vfio-ap: allow assignment of unavailable AP queues to mdev device

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, 8 Mar 2022 11:06:37 +0100
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 v18 08/18] s390/vfio-ap: allow assignment of unavailable
 AP queues to mdev device

On Mon, 7 Mar 2022 18:45:45 -0500
Tony Krowiak <akrowiak@...ux.ibm.com> wrote:

[..]
> >>> s/belong to the matrix_mdev's matrix/are fields of the matrix_mdev
> >>> object/  
> >> This is the comment I wrote:
> >>
> >>           /*
> >>            * Comparing an mdev's newly updated apm/aqm with itself would
> >>            * result in a false positive when verifying whether any APQNs
> >>            * are shared; so, if the input apm and aqm belong to the
> >>            * matrix_mdev's matrix, then move on to the next one.
> >>            */
> >>
> >> However, I'd be happy to change it to whatever either of you want.  
> > What ain't obvious for the comment is that "belong to" actually means
> > composition and not association. In other words, there there is no
> > pointer/indirection involved, a pointer that would tell us what matrix
> > does belong to what matrix_mdev, but rather the matrix is just a part
> > of the matrix_mdev object.
> >
> > I don't like 'false positive' either, and whether the apm/aqm is
> > newly updated or not is also redundant and confusing in my opinion. When
> > we check because of inuse there is not updated whatever. IMHO the old
> > message was better than this one.
> >
> > Just my opinion, if you two agree, that this is the way to go, I'm fine
> > with that.
> >
> > Regards,
> > Halil  
> 
> Feel free to recommend the verbiage for this comment. I'm not married
> to my comments and am open to anything that helps others to
> understand what is going on here. It seems obvious to me, but I wrote
> the code. Obviously, it is not so obvious based on Jason's comments,
> so maybe someone else can compose a better comment.

/*
* If the input apm and aqm are fields of the matrix_mdev object,
* then move on to the next matrix_mdev.
*/

Regards,
Halil