Hello Pekka,
Just a few updates to vmevents:
- Some cleanups and refactorings -- needed for easier integration of 'memory pressure' work; - Forward to newer Linus' tree, fix conflicts.
For convenience, the merge commit and all the patches can be pulled from this repo:
git://git.infradead.org/users/cbou/linux-vmevent.git tags/vmevent-updates
Thanks, Anton.
struct vmevent_watch_event and a few definitions are not used anywhere, so let's remove them.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- mm/vmevent.c | 10 ---------- 1 file changed, 10 deletions(-)
diff --git a/mm/vmevent.c b/mm/vmevent.c index a7f1042..39ef786 100644 --- a/mm/vmevent.c +++ b/mm/vmevent.c @@ -11,16 +11,6 @@ #include <linux/swap.h> #undef nr_swap_pages /* This is defined to a constant for SWAP=n case */
-#define VMEVENT_MAX_FREE_THRESHOD 100 - -#define VMEVENT_MAX_EATTR_ATTRS 64 - -struct vmevent_watch_event { - u64 nr_avail_pages; - u64 nr_free_pages; - u64 nr_swap_pages; -}; - struct vmevent_watch { struct vmevent_config config;
Soon we'll use this new function for other code; plus this makes code less indented.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- mm/vmevent.c | 107 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 57 insertions(+), 50 deletions(-)
diff --git a/mm/vmevent.c b/mm/vmevent.c index 39ef786..d434c11 100644 --- a/mm/vmevent.c +++ b/mm/vmevent.c @@ -77,6 +77,59 @@ enum { VMEVENT_ATTR_STATE_VALUE_WAS_GT = (1UL << 31), };
+static bool vmevent_match_attr(struct vmevent_attr *attr, u64 value) +{ + u32 state = attr->state; + bool attr_lt = state & VMEVENT_ATTR_STATE_VALUE_LT; + bool attr_gt = state & VMEVENT_ATTR_STATE_VALUE_GT; + bool attr_eq = state & VMEVENT_ATTR_STATE_VALUE_EQ; + bool edge = state & VMEVENT_ATTR_STATE_EDGE_TRIGGER; + u32 was_lt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_LT; + u32 was_gt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_GT; + bool lt = value < attr->value; + bool gt = value > attr->value; + bool eq = value == attr->value; + bool was_lt = state & was_lt_mask; + bool was_gt = state & was_gt_mask; + bool was_eq = was_lt && was_gt; + bool ret = false; + + if (!state) + return false; + + if (!attr_lt && !attr_gt && !attr_eq) + return false; + + if (((attr_lt && lt) || (attr_gt && gt) || (attr_eq && eq)) && !edge) + return true; + + if (attr_eq && eq && was_eq) { + return false; + } else if (attr_lt && lt && was_lt && !was_eq) { + return false; + } else if (attr_gt && gt && was_gt && !was_eq) { + return false; + } else if (eq) { + state |= was_lt_mask; + state |= was_gt_mask; + if (attr_eq) + ret = true; + } else if (lt) { + state |= was_lt_mask; + state &= ~was_gt_mask; + if (attr_lt) + ret = true; + } else if (gt) { + state |= was_gt_mask; + state &= ~was_lt_mask; + if (attr_gt) + ret = true; + } + + attr->state = state; + return ret; +} + static bool vmevent_match(struct vmevent_watch *watch) { struct vmevent_config *config = &watch->config; @@ -84,57 +137,11 @@ static bool vmevent_match(struct vmevent_watch *watch)
for (i = 0; i < config->counter; i++) { struct vmevent_attr *attr = &config->attrs[i]; - u32 state = attr->state; - bool attr_lt = state & VMEVENT_ATTR_STATE_VALUE_LT; - bool attr_gt = state & VMEVENT_ATTR_STATE_VALUE_GT; - bool attr_eq = state & VMEVENT_ATTR_STATE_VALUE_EQ; - - if (!state) - continue; + u64 val;
- if (attr_lt || attr_gt || attr_eq) { - bool edge = state & VMEVENT_ATTR_STATE_EDGE_TRIGGER; - u32 was_lt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_LT; - u32 was_gt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_GT; - u64 value = vmevent_sample_attr(watch, attr); - bool lt = value < attr->value; - bool gt = value > attr->value; - bool eq = value == attr->value; - bool was_lt = state & was_lt_mask; - bool was_gt = state & was_gt_mask; - bool was_eq = was_lt && was_gt; - bool ret = false; - - if (((attr_lt && lt) || (attr_gt && gt) || - (attr_eq && eq)) && !edge) - return true; - - if (attr_eq && eq && was_eq) { - return false; - } else if (attr_lt && lt && was_lt && !was_eq) { - return false; - } else if (attr_gt && gt && was_gt && !was_eq) { - return false; - } else if (eq) { - state |= was_lt_mask; - state |= was_gt_mask; - if (attr_eq) - ret = true; - } else if (lt) { - state |= was_lt_mask; - state &= ~was_gt_mask; - if (attr_lt) - ret = true; - } else if (gt) { - state |= was_gt_mask; - state &= ~was_lt_mask; - if (attr_gt) - ret = true; - } - - attr->state = state; - return ret; - } + val = vmevent_sample_attr(watch, attr); + if (vmevent_match_attr(attr, val)) + return true; }
return false;
On Thu, Oct 4, 2012 at 1:21 PM, Anton Vorontsov anton.vorontsov@linaro.org wrote:
Soon we'll use this new function for other code; plus this makes code less indented.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org
mm/vmevent.c | 107 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 57 insertions(+), 50 deletions(-)
diff --git a/mm/vmevent.c b/mm/vmevent.c index 39ef786..d434c11 100644 --- a/mm/vmevent.c +++ b/mm/vmevent.c @@ -77,6 +77,59 @@ enum { VMEVENT_ATTR_STATE_VALUE_WAS_GT = (1UL << 31), };
+static bool vmevent_match_attr(struct vmevent_attr *attr, u64 value) +{
u32 state = attr->state;bool attr_lt = state & VMEVENT_ATTR_STATE_VALUE_LT;bool attr_gt = state & VMEVENT_ATTR_STATE_VALUE_GT;bool attr_eq = state & VMEVENT_ATTR_STATE_VALUE_EQ;bool edge = state & VMEVENT_ATTR_STATE_EDGE_TRIGGER;u32 was_lt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_LT;u32 was_gt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_GT;bool lt = value < attr->value;bool gt = value > attr->value;bool eq = value == attr->value;bool was_lt = state & was_lt_mask;bool was_gt = state & was_gt_mask;
[snip]
So I merged this patch but vmevent_match_attr() is still too ugly for words. It really could use some serious cleanups.
On Fri, Oct 12, 2012 at 03:37:43PM +0300, Pekka Enberg wrote: [...]
+static bool vmevent_match_attr(struct vmevent_attr *attr, u64 value) +{
u32 state = attr->state;bool attr_lt = state & VMEVENT_ATTR_STATE_VALUE_LT;bool attr_gt = state & VMEVENT_ATTR_STATE_VALUE_GT;bool attr_eq = state & VMEVENT_ATTR_STATE_VALUE_EQ;bool edge = state & VMEVENT_ATTR_STATE_EDGE_TRIGGER;u32 was_lt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_LT;u32 was_gt_mask = VMEVENT_ATTR_STATE_VALUE_WAS_GT;bool lt = value < attr->value;bool gt = value > attr->value;bool eq = value == attr->value;bool was_lt = state & was_lt_mask;bool was_gt = state & was_gt_mask;[snip]
So I merged this patch but vmevent_match_attr() is still too ugly for words. It really could use some serious cleanups.
Thanks a lot for merging these cleanups!
Yes, the patch wasn't meant to simplify the matching logic, but just to let us use the function in other places.
I once started converting the function into table-based approach, but the code started growing, and I abandoned the idea for now. I might resume the work just for the fun of it, but the code will be larger than this ad-hoc function, althouh surely it will be more generic and understandable.
But let's solve primary problems with the vmevent first. :-)
Thanks, Anton.
Currently, we sample the same values in vmevent_sample() and vmevent_match(), but we can easily avoid this. Also saves loop iterations.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- mm/vmevent.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/mm/vmevent.c b/mm/vmevent.c index d434c11..d643615 100644 --- a/mm/vmevent.c +++ b/mm/vmevent.c @@ -133,18 +133,22 @@ static bool vmevent_match_attr(struct vmevent_attr *attr, u64 value) static bool vmevent_match(struct vmevent_watch *watch) { struct vmevent_config *config = &watch->config; + bool ret = 0; int i;
for (i = 0; i < config->counter; i++) { struct vmevent_attr *attr = &config->attrs[i]; + struct vmevent_attr *samp = &watch->sample_attrs[i]; u64 val;
val = vmevent_sample_attr(watch, attr); - if (vmevent_match_attr(attr, val)) - return true; + if (!ret && vmevent_match_attr(attr, val)) + ret = 1; + + samp->value = val; }
- return false; + return ret; }
/* @@ -161,20 +165,11 @@ static bool vmevent_match(struct vmevent_watch *watch) */ static void vmevent_sample(struct vmevent_watch *watch) { - int i; - if (atomic_read(&watch->pending)) return; if (!vmevent_match(watch)) return;
- for (i = 0; i < watch->nr_attrs; i++) { - struct vmevent_attr *attr = &watch->sample_attrs[i]; - - attr->value = vmevent_sample_attr(watch, - watch->config_attrs[i]); - } - atomic_set(&watch->pending, 1); }
Hi Anton,
On Thu, Oct 4, 2012 at 1:20 PM, Anton Vorontsov anton.vorontsov@linaro.org wrote:
Hello Pekka,
Just a few updates to vmevents:
- Some cleanups and refactorings -- needed for easier integration of 'memory pressure' work;
- Forward to newer Linus' tree, fix conflicts.
Applied, thanks!
linaro-kernel@lists.linaro.org
-
Anton Vorontsov -
Pekka Enberg