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@xxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxx>
Thanks!
Eric
