Hi,

Just couple of observations, which maybe wrong below

-----Original Message-----
From: Pekka Enberg [mailto:penberg@gmail.com] On Behalf Of ext Pekka
Enberg
Sent: 17 January, 2012 20:51

....

+struct vmnotify_config {
+	/*
+	 * Size of the struct for ABI extensibility.
+	 */
+	__u32		   size;
+
+	/*
+	 * Notification type bitmask
+	 */
+	__u64			type;
+
+	/*
+	 * Free memory threshold in percentages [1..99]
+	 */
+	__u32			free_threshold;

Would be possible to not use percents for thesholds? Accounting in pages even not so difficult to user-space.
Also, looking on vmnotify_match I understand that events propagated to user-space only in case threshold trigger change state from 0 to 1 but not back, 1-> 0 is very useful event as well.

Would be possible to use for threshold pointed value(s) e.g. according to enum zone_state_item, because kinds of memory to track could be different?
E.g. to tracking paging activity NR_ACTIVE_ANON and NR_ACTIVE_FILE could be interesting, not only free.

+
+	/*
+	 * Sample period in nanoseconds
+	 */
+	__u64			sample_period_ns;
+};
+

....

+struct vmnotify_event {
+	/* Size of the struct for ABI extensibility. */
+	__u32			size;
+
+	__u64			nr_avail_pages;
+
+	__u64			nr_swap_pages;
+
+	__u64			nr_free_pages;
+};

Two fields here most likely session-constant, (nr_avail_pages and nr_swap_pages), seems not much sense to report them in every event.
If we have memory/swap hotplug user-space can use sysinfo() call.

+static void vmnotify_sample(struct vmnotify_watch *watch) {

...

+	si_meminfo(&si);
+	event.nr_avail_pages	= si.totalram;
+
+#ifdef CONFIG_SWAP
+	si_swapinfo(&si);
+	event.nr_swap_pages	= si.totalswap;
+#endif
+

Why not to use global_page_state() directly? si_meminfo() and especial si_swapinfo are quite expensive call.

+static void vmnotify_start_timer(struct vmnotify_watch *watch) {
+	u64 sample_period = watch->config.sample_period_ns;
+
+	hrtimer_init(&watch->timer, CLOCK_MONOTONIC,
HRTIMER_MODE_REL);
+	watch->timer.function = vmnotify_timer_fn;
+
+	hrtimer_start(&watch->timer, ns_to_ktime(sample_period),
+HRTIMER_MODE_REL_PINNED); }

Do I understand correct you allocate timer for every user-space client and propagate events every pointed interval?
What will happened with system if we have a timer but need to turn CPU off? The timer must not be a reason to wakeup if user-space is sleeping.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>