Re: [Xen-devel] [PATCH-for-4.13 v4] Rationalize max_grant_frames and max_maptrack_frames handling

["Durrant, Paul" <pdurrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 28 November 2019 16:37
> To: Durrant, Paul <pdurrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Anthony PERARD <anthony.perard@xxxxxxxxxx>;
> George Dunlap <george.dunlap@xxxxxxxxxx>; Roger Pau Monné
> <roger.pau@xxxxxxxxxx>; Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>;
> George Dunlap <George.Dunlap@xxxxxxxxxxxxx>; Ian Jackson
> <ian.jackson@xxxxxxxxxxxxx>; Marek Marczykowski-Górecki
> <marmarek@xxxxxxxxxxxxxxxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>;
> Julien Grall <julien@xxxxxxx>; Wei Liu <wl@xxxxxxx>
> Subject: Re: [PATCH-for-4.13 v4] Rationalize max_grant_frames and
> max_maptrack_frames handling
> 
> On 28.11.2019 14:58, Paul Durrant wrote:
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -84,11 +84,43 @@ struct grant_table {
> >      struct grant_table_arch arch;
> >  };
> >
> > +static int __init parse_gnttab_limit(const char *param, const char
> *arg,
> 
> No __init please here and below, for this being runtime option
> parsing functions.
> 

Sorry, yes... forgot about the runtime part.

> > +                                     unsigned int *valp)
> > +{
> > +    const char *e;
> > +    unsigned long val;
> > +
> > +    val = simple_strtoul(arg, &e, 0);
> > +    if ( *e )
> > +        return -EINVAL;
> > +
> > +    if ( val <= INT_MAX )
> > +        *valp = val;
> > +    else
> > +        printk("parameter \"%s\" value \"%s\" is out of range; using
> value \"%u\"\n",
> > +               param, arg, *valp);
> 
> Better store INT_MAX in this case rather than leaving the value
> unchanged? Or otherwise ...
> 
> > +    return 0;
> 
> ... at least don't return success?

TBH I wasn't sure what the best thing to do was. In the end I opted for the 
warning and a successful completion as I thought a failure would be largely 
unhelpful. I can change this into an error though.

> 
> > +}
> > +
> >  unsigned int __read_mostly opt_max_grant_frames = 64;
> > -integer_runtime_param("gnttab_max_frames", opt_max_grant_frames);
> > +
> > +static int __init parse_gnttab_max_frames(const char *arg)
> > +{
> > +    return parse_gnttab_limit("gnttab_max_frames", arg,
> > +                              &opt_max_grant_frames);
> > +}
> > +custom_runtime_param("gnttab_max_frames", parse_gnttab_max_frames);
> >
> >  unsigned int __read_mostly opt_max_maptrack_frames = 1024;
> 
> As indicated this wants to become static now.

Sorry I forgot about that.

  Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel