| Message ID | 20191211042657.6037-1-sjpark@amazon.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xenbus/backend: Add a memory pressure handler callback | expand |
On Wed, Dec 11, 2019 at 04:26:57AM +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 ^ s/the// > memory is challenging for large systems having dynamic resource > utilization patterns. Also, such a static configuration might lack > flexibility. > > To mitigate such problems, this commit adds a memory reclaim callback to > 'xenbus_driver'. If a memory pressure is detected, 'xenbus' requests ^ s/a// > every backend driver to volunarily release its memory. > > Note that it would be able to improve the callback facility for more ^ possible > sophisticated handlings of general pressures. For example, it would be ^ handling of resource starvation. > possible to monitor the memory consumption of each device and issue the > release requests to only devices which causing the pressure. Also, the > callback could be extended to handle not only memory, but general > resources. Nevertheless, this version of the implementation defers such > sophisticated goals as a future work. > > Reviewed-by: Juergen Gross <jgross@suse.com> > Signed-off-by: SeongJae Park <sjpark@amazon.de> > --- > drivers/xen/xenbus/xenbus_probe_backend.c | 32 +++++++++++++++++++++++ > include/xen/xenbus.h | 1 + > 2 files changed, 33 insertions(+) > > diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c b/drivers/xen/xenbus/xenbus_probe_backend.c > index b0bed4faf44c..aedbe2198de5 100644 > --- a/drivers/xen/xenbus/xenbus_probe_backend.c > +++ b/drivers/xen/xenbus/xenbus_probe_backend.c > @@ -248,6 +248,35 @@ static int backend_probe_and_watch(struct notifier_block *notifier, > return NOTIFY_DONE; > } > > +static int xenbus_backend_reclaim(struct device *dev, void *data) No need for the xenbus_ prefix since it's a static function, ie: backend_reclaim_memory should be fine IMO. > +{ > + struct xenbus_driver *drv; I've asked for this variable to be constified in v5, is it not possible to make it const? > + > + if (!dev->driver) > + return 0; > + drv = to_xenbus_driver(dev->driver); > + if (drv && drv->reclaim) > + drv->reclaim(to_xenbus_device(dev)); > + 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 = { I would drop the xenbus prefix, and I think it's not possible to constify this due to register_shrinker expecting a non-const parameter? > + .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 +293,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"); Can you add a xenbus prefix to the error message? Or else it's hard to know which subsystem is complaining when you see such message on the log. ie: "xenbus: shrinker ..." > + > return 0; > } > subsys_initcall(xenbus_probe_backend_init); > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h > index 869c816d5f8c..196260017666 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); > + void (*reclaim)(struct xenbus_device *dev); reclaim_memory (if Juergen agrees). Thanks, Roger.
On 11.12.19 12:46, Roger Pau Monné wrote: > On Wed, Dec 11, 2019 at 04:26:57AM +0000, SeongJae Park wrote: >> + >> return 0; >> } >> subsys_initcall(xenbus_probe_backend_init); >> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h >> index 869c816d5f8c..196260017666 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); >> + void (*reclaim)(struct xenbus_device *dev); > > reclaim_memory (if Juergen agrees). I do agree. Juergen
diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c b/drivers/xen/xenbus/xenbus_probe_backend.c index b0bed4faf44c..aedbe2198de5 100644 --- a/drivers/xen/xenbus/xenbus_probe_backend.c +++ b/drivers/xen/xenbus/xenbus_probe_backend.c @@ -248,6 +248,35 @@ 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; + + if (!dev->driver) + return 0; + drv = to_xenbus_driver(dev->driver); + if (drv && drv->reclaim) + drv->reclaim(to_xenbus_device(dev)); + 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 +293,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..196260017666 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); + void (*reclaim)(struct xenbus_device *dev); }; static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)