Re: [PATCH v1 08/16] vfio/ccw: pass page count to page_array struct

[Eric Farman <farman@linux.ibm.com>]

On Fri, 2022-12-16 at 14:59 -0500, Matthew Rosato wrote:
> On 11/21/22 4:40 PM, Eric Farman wrote:
> > The allocation of our page_array struct calculates the number
> > of 4K pages that would be needed to hold a certain number of
> > bytes. But, since the number of pages that will be pinned is
> > also calculated by the length of the IDAL, this logic is
> > unnecessary. Let's pass that information in directly, and
> > avoid the math within the allocator.
> > 
> > Also, let's make this two allocations instead of one,
> > to make it apparent what's happening within here.
> > 
> > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > ---
> >  drivers/s390/cio/vfio_ccw_cp.c | 23 +++++++++++++----------
> >  1 file changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/s390/cio/vfio_ccw_cp.c
> > b/drivers/s390/cio/vfio_ccw_cp.c
> > index 4b6b5f9dc92d..66e890441163 100644
> > --- a/drivers/s390/cio/vfio_ccw_cp.c
> > +++ b/drivers/s390/cio/vfio_ccw_cp.c
> > @@ -43,7 +43,7 @@ struct ccwchain {
> >   * page_array_alloc() - alloc memory for page array
> >   * @pa: page_array on which to perform the operation
> >   * @iova: target guest physical address
> > - * @len: number of bytes that should be pinned from @iova
> > + * @len: number of pages that should be pinned from @iova
> >   *
> >   * Attempt to allocate memory for page array.
> >   *
> > @@ -63,18 +63,20 @@ static int page_array_alloc(struct page_array
> > *pa, u64 iova, unsigned int len)
> >         if (pa->pa_nr || pa->pa_iova)
> >                 return -EINVAL;
> >  
> > -       pa->pa_nr = ((iova & ~PAGE_MASK) + len + (PAGE_SIZE - 1))
> > >> PAGE_SHIFT;
> > -       if (!pa->pa_nr)
> > +       if (!len)
> 
> Seems like a weird way to check this.  if (len == 0) ?

Yeah... Weird before, weird now. I'll fix this up.

> 
> Otherwise:
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

Thanks!

Eric