Re: [PATCH RESEND v5 09/24] x86: refactor psr: set value: assemble features value array.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Feb 07, 2017 at 10:51:27AM +0800, Yi Sun wrote:
> On 17-01-31 15:57:26, Konrad Rzeszutek Wilk wrote:
> > >  static int assemble_val_array(uint64_t *val,
> > > @@ -550,7 +641,25 @@ static int assemble_val_array(uint64_t *val,
> > >                                const struct psr_socket_info *info,
> > >                                unsigned int old_cos)
> > >  {
> > > -    return -EINVAL;
> > > +    const struct feat_node *feat;
> > > +    int ret;
> > > +    uint64_t *val_tmp = val;
> > > +
> > > +    if ( !val )
> > > +        return -EINVAL;
> > > +
> > > +    /* Get all features current values according to old_cos. */
> > > +    list_for_each_entry(feat, &info->feat_list, list)
> > > +    {
> > > +        /* value getting order is same as feature list */
> > > +        ret = feat->ops.get_old_val(val_tmp, feat, old_cos);
> > 
> > Shouldn't we check ret for negative values?
> > > +
> > > +        val_tmp += feat->ops.get_cos_num(feat);
> > > +        if ( val_tmp - val > array_len)
> > > +            return -EINVAL;
> > 
> > 
> > Perhaps: ENOSPC ?
> > 
> > Also this function does do any assembling. It just verifies.
> > Perhaps other patches add extra work here? In which case you may
> > want to mention that in the commit description.
> > 
> Array "assembling" is done here. But maybe the "assembling" is not accurate.
> In this function, I want to get all features old values and put them into
> the array according to every feature's position in feature list.

combine?
> 
> Do you have more accurate word than "assemble"? Thanks!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel