Re: [PATCH v10 09/16] s390/vfio-ap: allow assignment of unavailable AP queues to mdev device

[Halil Pasic <pasic@linux.ibm.com>]

On Wed, 30 Sep 2020 08:59:36 -0400
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> >> @@ -572,6 +455,11 @@ static int vfio_ap_mdev_verify_no_sharing(struct ap_matrix_mdev *matrix_mdev,
> >>   	DECLARE_BITMAP(aqm, AP_DOMAINS);
> >>   
> >>   	list_for_each_entry(lstdev, &matrix_dev->mdev_list, node) {
> >> +		/*
> >> +		 * If either of the input masks belongs to the mdev to which an
> >> +		 * AP resource is being assigned, then we don't need to verify
> >> +		 * that mdev's masks.
> >> +		 */
> >>   		if (matrix_mdev == lstdev)
> >>   			continue;
> >>     
> > Seems unrelated.  
> 
> What seems unrelated? The matrix_mdev passed in is the mdev to which 
> assignment is
> being made. This function is verifying that no APQN assigned to the 
> matrix_mdev is
> assigned to any other mdev. Since we are looping through all mdevs here, 
> we are
> skipping the verification if the current mdev being examined is the same 
> as the matrix_mdev
> to which the assignment is being made. Maybe I'm not understanding your 
> point here.

Sorry I did not explain myself clear enough. By seems unrelated, I mean
that while valid possibly it does not contribute towards achieving the
objective of the patch (as stated by the commit message and the short
description).

AFAICT this is about documenting some pre-existing logic that is not
changed, nor it needs to be changed to achieve the objective.

This patch does not change the function at all (except for the
comment). If the comment is about the new arguments, then is
belongs to "implement in-use callback for vfio_ap driver" where those
were added.

BTW I find the comment hard to understand because I don't see "If either
of the input masks belongs to the mdev to which an  AP resource is being
assigned expressed in the code.

I would rather have the docstring of the function updated so the
relationship of the three arguments is clear.

Regards,
Halil