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 <[email protected]>
---
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;
--
1.7.12.1
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 <[email protected]>
---
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);
}
--
1.7.12.1
Soon we'll use this new function for other code; plus this makes code less
indented.
Signed-off-by: Anton Vorontsov <[email protected]>
---
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;
--
1.7.12.1
Hi Anton,
On Thu, Oct 4, 2012 at 1:20 PM, Anton Vorontsov
<[email protected]> 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!
On Thu, Oct 4, 2012 at 1:21 PM, Anton Vorontsov
<[email protected]> wrote:
> Soon we'll use this new function for other code; plus this makes code less
> indented.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
> 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.