On 7/2/19 9:58 AM, Farhan Ali wrote: > > > On 07/02/2019 04:42 AM, Cornelia Huck wrote: >> On Mon, 1 Jul 2019 12:23:44 -0400 >> Farhan Ali <alifm@xxxxxxxxxxxxx> wrote: >> >>> We don't set cp->initialized to true so calling cp_free >>> will just return and not do anything. >>> >>> Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx> >>> --- >>> drivers/s390/cio/vfio_ccw_cp.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/drivers/s390/cio/vfio_ccw_cp.c >>> b/drivers/s390/cio/vfio_ccw_cp.c >>> index 5ac4c1e..cab1be9 100644 >>> --- a/drivers/s390/cio/vfio_ccw_cp.c >>> +++ b/drivers/s390/cio/vfio_ccw_cp.c >>> @@ -647,8 +647,6 @@ int cp_init(struct channel_program *cp, struct >>> device *mdev, union orb *orb) >>> /* Build a ccwchain for the first CCW segment */ >>> ret = ccwchain_handle_ccw(orb->cmd.cpa, cp); >>> - if (ret) >>> - cp_free(cp); >> >> Makes sense; hopefully ccwchain_handle_ccw() cleans up correctly on >> error :) (I think it does) >> > > I have checked that it does as well, but wouldn't hurt if someone else > also glances over once again :) Oh noes. What happens once we start encountering TICs? If we do: ccwchain_handle_ccw() (OK) ccwchain_loop_tic() (OK) ccwchain_handle_ccw() (FAIL) The first _handle_ccw() will have added a ccwchain to the cp list, which doesn't appear to get cleaned up now. That used to be done in cp_init() until I squashed cp_free and cp_unpin_free. :( > >> Maybe add a comment >> >> /* ccwchain_handle_ccw() already cleans up on error */ >> >> so we don't stumble over this in the future? > > Sure. > >> >> (Also, does this want a Fixes: tag?) > > This might warrant a fixes tag as well. >> >>> if (!ret) >>> cp->initialized = true; >> >>
