On Tue, Dec 10, 2019 at 08:06:27AM +0000, SeongJae Park wrote: > Granting pages consumes backend system memory. In systems configured > with insufficient spare memory for those pages, it can cause a memory > pressure situation. However, finding the optimal amount of the spare > memory is challenging for large systems having dynamic resource > utilization patterns. Also, such a static configuration might lack a
s/lack a/lack/ > flexibility. > > To mitigate such problems, this commit adds a memory reclaim callback to > 'xenbus_driver'. Using this facility, 'xenbus' would be able to monitor > a memory pressure and request specific devices of specific backend s/monitor a/monitor/ > drivers which causing the given pressure to voluntarily release its ...which are causing... > memory. > > That said, this commit simply requests every callback registered driver > to release its memory for every domain, rather than issueing the s/issueing/issuing/ > requests to the drivers and the domain in charge. Such things will be I'm afraid I don't understand the "domain in charge" part of this sentence. > done in a futur. Also, this commit focuses on memory only. However, it ... done in a future change. Also I think the period after only should be removed in order to tie both sentences together. > would be ablt to be extended for general resources. s/ablt/able/ > > Signed-off-by: SeongJae Park <sjp...@amazon.de> > --- > drivers/xen/xenbus/xenbus_probe_backend.c | 31 +++++++++++++++++++++++ > include/xen/xenbus.h | 1 + > 2 files changed, 32 insertions(+) > > diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c > b/drivers/xen/xenbus/xenbus_probe_backend.c > index b0bed4faf44c..5a5ba29e39df 100644 > --- a/drivers/xen/xenbus/xenbus_probe_backend.c > +++ b/drivers/xen/xenbus/xenbus_probe_backend.c > @@ -248,6 +248,34 @@ static int backend_probe_and_watch(struct notifier_block > *notifier, > return NOTIFY_DONE; > } > > +static int xenbus_backend_reclaim(struct device *dev, void *data) > +{ > + struct xenbus_driver *drv; Newline and const. > + if (!dev->driver) > + return -ENOENT; > + drv = to_xenbus_driver(dev->driver); > + if (drv && drv->reclaim) > + drv->reclaim(to_xenbus_device(dev)); You seem to completely ignore the return of the reclaim hook... > + return 0; > +} > + > +/* > + * Returns 0 always because we are using shrinker to only detect memory > + * pressure. > + */ > +static unsigned long xenbus_backend_shrink_count(struct shrinker *shrinker, > + struct shrink_control *sc) > +{ > + bus_for_each_dev(&xenbus_backend.bus, NULL, NULL, > + xenbus_backend_reclaim); > + return 0; > +} > + > +static struct shrinker xenbus_backend_shrinker = { > + .count_objects = xenbus_backend_shrink_count, > + .seeks = DEFAULT_SEEKS, > +}; > + > static int __init xenbus_probe_backend_init(void) > { > static struct notifier_block xenstore_notifier = { > @@ -264,6 +292,9 @@ static int __init xenbus_probe_backend_init(void) > > register_xenstore_notifier(&xenstore_notifier); > > + if (register_shrinker(&xenbus_backend_shrinker)) > + pr_warn("shrinker registration failed\n"); > + > return 0; > } > subsys_initcall(xenbus_probe_backend_init); > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h > index 869c816d5f8c..cdb075e4182f 100644 > --- a/include/xen/xenbus.h > +++ b/include/xen/xenbus.h > @@ -104,6 +104,7 @@ struct xenbus_driver { > struct device_driver driver; > int (*read_otherend_details)(struct xenbus_device *dev); > int (*is_ready)(struct xenbus_device *dev); > + unsigned (*reclaim)(struct xenbus_device *dev); ... hence I wonder why it's returning an unsigned when it's just ignored. IMO it should return an int to signal errors, and the return should be ignored. Also, I think it would preferable for this function to take an extra parameter to describe the resource the driver should attempt to free (ie: memory or interrupts for example). I'm however not able to find any existing Linux type to describe such resources. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel