Re: [PATCH 1/4] vfio: ccw: fix cleanup if cp_prefetch fails

[Halil Pasic]

ping

I think get this fixed. Better sooner than later.

On 03/27/2018 03:42 AM, Dong Jia Shi wrote:
> * Cornelia Huck <cohuck@xxxxxxxxxx> [2018-03-26 14:28:39 +0200]:
>
> [...]
>
> > > By the way, since you will propose a new version,
> > > you have a long description of the cp_prefetch function in the code.
> > > I think you should modify it according to the changes and describe how and
> > > why the ch_len field of each chain is used and changed by this function.
> > >
> > > Something like:
> > >
> > > "
> > > For each chain composing the channel program:
> > > On entry ch_len hold the count of CCW to be translated.
> >
> > s/hold/holds/ ?
> Sure.
>
> > > On exit ch_len is adjusted to the count of successfully translated CCW.
> > >
> > > This allows cp_free to find in ch_len the count of CCW to free in a chain.
> > > "
> > >
> > > Could also be inside the commit message.
> > >
> > > Pierre
> > >
> > >
> > > >   
> > > > > > Acked-by: Pierre Morel <pmorel@xxxxxxxxxxxxxxxxxx>
> > > > > > Reviewed-by: Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx>
> > > > > > Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxxxxxxx>
> > > > > > Signed-off-by: Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx>
> > > > > > ---
> > > > > >    drivers/s390/cio/vfio_ccw_cp.c | 9 ++++++++-
> > > > > >    1 file changed, 8 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c
> > > > > > index d9a2fffd034b..2be114db02f9 100644
> > > > > > --- a/drivers/s390/cio/vfio_ccw_cp.c
> > > > > > +++ b/drivers/s390/cio/vfio_ccw_cp.c
> > > > > > @@ -749,11 +749,18 @@ int cp_prefetch(struct channel_program *cp)
> > > > > >    		for (idx = 0; idx < len; idx++) {
> > > > > >    			ret = ccwchain_fetch_one(chain, idx, cp);
> > > > > >    			if (ret)
> > > > > > -				return ret;
> > > > > > +				goto out_err;
> > > > > >    		}
> > > > > >    	}
> > > > > >
> > > > > >    	return 0;
> > > > > > +out_err:
> > > > > > +	/* Only cleanup the chain elements that where actually translated. */
> >
> > s/where/were/
> Ok.
>
> > > > > > +	chain->ch_len = idx;
> > > > > > +	list_for_each_entry_continue(chain, &cp->ccwchain_list, next) {
> > > > > > +		chain->ch_len = 0;
> > > > > > +	}
> > > > > > +	return ret;
> > > > > >    }
> > > > > >
> > > > > >    /**
> > > > > >