On Fri, 9 Nov 2018 09:49:22 -0500 Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > > On 11/09/2018 05:45 AM, Cornelia Huck wrote: > > On Fri, 9 Nov 2018 03:39:28 +0100 > > Eric Farman <farman@xxxxxxxxxxxxx> wrote: > > > >> If pfn_array_alloc fails somehow, we need to release the > >> pfn_array_table that was malloc'd earlier. > >> > >> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx> > >> --- > >> drivers/s390/cio/vfio_ccw_cp.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/s390/cio/vfio_ccw_cp.c > >> b/drivers/s390/cio/vfio_ccw_cp.c index fd77e46eb3b2..ef5ab45d94b3 > >> 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c > >> +++ b/drivers/s390/cio/vfio_ccw_cp.c > >> @@ -528,7 +528,7 @@ static int ccwchain_fetch_direct(struct > >> ccwchain *chain, > >> ret = pfn_array_alloc_pin(pat->pat_pa, cp->mdev, > >> ccw->cda, ccw->count); if (ret < 0) > >> - goto out_init; > >> + goto out_unpin; > >> > >> /* Translate this direct ccw to a idal ccw. */ > >> idaws = kcalloc(ret, sizeof(*idaws), GFP_DMA | > >> GFP_KERNEL); > > > > It's a bit confusing that this will also call vfio_unpin_pages() > > even though there should not be any pinned pages at that point in > > time; but from what I see, it should not hurt, so this patch is > > fine. > > > > Yeah, confusing indeed. The problem today is that we don't undo the > pfn_array_table_init() call that happened prior to this error, and so > there would be a leak of the pat->pat_pa memory. So going to > out_init is certainly not right. > > In the pfn_array patches later, I do conditionally call > vfio_unpin_pages() based on the contents of pfn_array->pa_iova, to > avoid the scenario you describe. > > - Eric > Quite confusing and generally awful. I will try to figure out the before-after on a series level, and then consider the individual patches in detail. The in between states are predestined to look awful because of the current state. Regards, Halil
