[patch] HT scheduler, sched-2.5.68-A9

[patch] HT scheduler, sched-2.5.68-A9

[Ingo Molnar]

the attached patch (against 2.5.68 or BK-curr) is the latest
implementation of the "no compromises" HT-scheduler. I fixed a couple of
bugs, and the scheduler is now stable and behaves properly on a
2-CPU-2-sibling HT testbox. The patch can also be downloaded from:

	http://redhat.com/~mingo/O(1)-scheduler/

bug reports, suggestions welcome,

	Ingo

--- linux/include/linux/sched.h.orig	
+++ linux/include/linux/sched.h	
@@ -147,6 +147,24 @@ typedef struct task_struct task_t;
 extern void sched_init(void);
 extern void init_idle(task_t *idle, int cpu);
 
+/*
+ * Is there a way to do this via Kconfig?
+ */
+#if CONFIG_NR_SIBLINGS_2
+# define CONFIG_NR_SIBLINGS 2
+#elif CONFIG_NR_SIBLINGS_4
+# define CONFIG_NR_SIBLINGS 4
+#else
+# define CONFIG_NR_SIBLINGS 0
+#endif
+
+#if CONFIG_NR_SIBLINGS
+# define CONFIG_SHARE_RUNQUEUE 1
+#else
+# define CONFIG_SHARE_RUNQUEUE 0
+#endif
+extern void sched_map_runqueue(int cpu1, int cpu2);
+
 extern void show_state(void);
 extern void show_trace(unsigned long *stack);
 extern void show_stack(unsigned long *stack);
@@ -814,11 +832,6 @@ static inline unsigned int task_cpu(stru
 	return p->thread_info->cpu;
 }
 
-static inline void set_task_cpu(struct task_struct *p, unsigned int cpu)
-{
-	p->thread_info->cpu = cpu;
-}
-
 #else
 
 static inline unsigned int task_cpu(struct task_struct *p)
@@ -826,10 +839,6 @@ static inline unsigned int task_cpu(stru
 	return 0;
 }
 
-static inline void set_task_cpu(struct task_struct *p, unsigned int cpu)
-{
-}
-
 #endif /* CONFIG_SMP */
 
 #endif /* __KERNEL__ */
--- linux/include/asm-i386/apic.h.orig	
+++ linux/include/asm-i386/apic.h	
@@ -101,4 +101,6 @@ extern unsigned int nmi_watchdog;
 
 #endif /* CONFIG_X86_LOCAL_APIC */
 
+extern int phys_proc_id[NR_CPUS];
+
 #endif /* __ASM_APIC_H */
--- linux/arch/i386/kernel/cpu/proc.c.orig	
+++ linux/arch/i386/kernel/cpu/proc.c	
@@ -1,4 +1,5 @@
 #include <linux/smp.h>
+#include <linux/sched.h>
 #include <linux/timex.h>
 #include <linux/string.h>
 #include <asm/semaphore.h>
@@ -114,6 +115,13 @@ static int show_cpuinfo(struct seq_file 
 		     fpu_exception ? "yes" : "no",
 		     c->cpuid_level,
 		     c->wp_works_ok ? "yes" : "no");
+#if CONFIG_SHARE_RUNQUEUE
+{
+	extern long __rq_idx[NR_CPUS];
+
+	seq_printf(m, "\nrunqueue\t: %ld\n", __rq_idx[n]);
+}
+#endif
 
 	for ( i = 0 ; i < 32*NCAPINTS ; i++ )
 		if ( test_bit(i, c->x86_capability) &&
--- linux/arch/i386/kernel/smpboot.c.orig	
+++ linux/arch/i386/kernel/smpboot.c	
@@ -38,6 +38,7 @@
 #include <linux/kernel.h>
 
 #include <linux/mm.h>
+#include <linux/sched.h>
 #include <linux/kernel_stat.h>
 #include <linux/smp_lock.h>
 #include <linux/irq.h>
@@ -933,6 +934,16 @@ void *xquad_portio;
 
 int cpu_sibling_map[NR_CPUS] __cacheline_aligned;
 
+static int test_ht;
+
+static int __init ht_setup(char *str)
+{
+	test_ht = 1;
+	return 1;
+}
+
+__setup("test_ht", ht_setup);
+
 static void __init smp_boot_cpus(unsigned int max_cpus)
 {
 	int apicid, cpu, bit;
@@ -1074,7 +1085,20 @@ static void __init smp_boot_cpus(unsigne
 	Dprintk("Boot done.\n");
 
 	/*
-	 * If Hyper-Threading is avaialble, construct cpu_sibling_map[], so
+	 * Here we can be sure that there is an IO-APIC in the system. Let's
+	 * go and set it up:
+	 */
+	smpboot_setup_io_apic();
+
+	setup_boot_APIC_clock();
+
+	/*
+	 * Synchronize the TSC with the AP
+	 */
+	if (cpu_has_tsc && cpucount && cpu_khz)
+		synchronize_tsc_bp();
+	/*
+	 * If Hyper-Threading is available, construct cpu_sibling_map[], so
 	 * that we can tell the sibling CPU efficiently.
 	 */
 	if (cpu_has_ht && smp_num_siblings > 1) {
@@ -1083,7 +1107,8 @@ static void __init smp_boot_cpus(unsigne
 		
 		for (cpu = 0; cpu < NR_CPUS; cpu++) {
 			int 	i;
-			if (!test_bit(cpu, &cpu_callout_map)) continue;
+			if (!test_bit(cpu, &cpu_callout_map))
+				continue;
 
 			for (i = 0; i < NR_CPUS; i++) {
 				if (i == cpu || !test_bit(i, &cpu_callout_map))
@@ -1099,17 +1124,41 @@ static void __init smp_boot_cpus(unsigne
 				printk(KERN_WARNING "WARNING: No sibling found for CPU %d.\n", cpu);
 			}
 		}
+#if CONFIG_SHARE_RUNQUEUE
+		/*
+		 * At this point APs would have synchronised TSC and
+		 * waiting for smp_commenced, with their APIC timer
+		 * disabled. So BP can go ahead do some initialization
+		 * for Hyper-Threading (if needed).
+		 */
+		for (cpu = 0; cpu < NR_CPUS; cpu++) {
+			int i;
+			if (!test_bit(cpu, &cpu_callout_map))
+				continue;
+			for (i = 0; i < NR_CPUS; i++) {
+				if (i <= cpu)
+					continue;
+				if (!test_bit(i, &cpu_callout_map))
+					continue;
+  
+				if (phys_proc_id[cpu] != phys_proc_id[i])
+					continue;
+				/*
+				 * merge runqueues, resulting in one
+				 * runqueue per package:
+				 */
+				sched_map_runqueue(cpu, i);
+				break;
+			}
+		}
+#endif
 	}
-
-	smpboot_setup_io_apic();
-
-	setup_boot_APIC_clock();
-
-	/*
-	 * Synchronize the TSC with the AP
-	 */
-	if (cpu_has_tsc && cpucount && cpu_khz)
-		synchronize_tsc_bp();
+#if CONFIG_SHARE_RUNQUEUE
+	if (smp_num_siblings == 1 && test_ht) {
+		printk("Simulating shared runqueues - mapping CPU#1's runqueue to CPU#0's runqueue.\n");
+		sched_map_runqueue(0, 1);
+	}
+#endif
 }
 
 /* These are wrappers to interface to the new boot process.  Someone
--- linux/arch/i386/Kconfig.orig	
+++ linux/arch/i386/Kconfig	
@@ -413,6 +413,24 @@ config SMP
 
 	  If you don't know what to do here, say N.
 
+choice
+
+	prompt "Hyperthreading Support"
+	depends on SMP
+	default NR_SIBLINGS_0
+
+config NR_SIBLINGS_0
+	bool "off"
+
+config NR_SIBLINGS_2
+	bool "2 siblings"
+
+config NR_SIBLINGS_4
+	bool "4 siblings"
+
+endchoice
+
+
 config PREEMPT
 	bool "Preemptible Kernel"
 	help
--- linux/kernel/sched.c.orig	
+++ linux/kernel/sched.c	
@@ -73,6 +73,7 @@
 #define INTERACTIVE_DELTA	2
 #define MAX_SLEEP_AVG		(10*HZ)
 #define STARVATION_LIMIT	(10*HZ)
+#define AGRESSIVE_IDLE_STEAL	1
 #define NODE_THRESHOLD		125
 
 /*
@@ -147,6 +148,48 @@ struct prio_array {
 };
 
 /*
+ * It's possible for two CPUs to share the same runqueue.
+ * This makes sense if they eg. share caches.
+ *
+ * We take the common 1:1 (SMP, UP) case and optimize it,
+ * the rest goes via remapping: rq_idx(cpu) gives the
+ * runqueue on which a particular cpu is on, cpu_idx(cpu)
+ * gives the rq-specific index of the cpu.
+ *
+ * (Note that the generic scheduler code does not impose any
+ *  restrictions on the mappings - there can be 4 CPUs per
+ *  runqueue or even assymetric mappings.)
+ */
+#if CONFIG_SHARE_RUNQUEUE
+# define MAX_NR_SIBLINGS CONFIG_NR_SIBLINGS
+  long __rq_idx[NR_CPUS] __cacheline_aligned;
+  static long __cpu_idx[NR_CPUS] __cacheline_aligned;
+# define rq_idx(cpu) (__rq_idx[(cpu)])
+# define cpu_idx(cpu) (__cpu_idx[(cpu)])
+# define for_each_sibling(idx, rq) \
+		for ((idx) = 0; (idx) < (rq)->nr_cpus; (idx)++)
+# define rq_nr_cpus(rq) ((rq)->nr_cpus)
+# define cpu_active_balance(c) (cpu_rq(c)->cpu[0].active_balance)
+#else
+# define MAX_NR_SIBLINGS 1
+# define rq_idx(cpu) (cpu)
+# define cpu_idx(cpu) 0
+# define for_each_sibling(idx, rq) while (0)
+# define cpu_active_balance(c) 0
+# define do_active_balance(rq, cpu) do { } while (0)
+# define rq_nr_cpus(rq) 1
+  static inline void active_load_balance(runqueue_t *rq, int this_cpu) { }
+#endif
+
+typedef struct cpu_s {
+	task_t *curr, *idle;
+	task_t *migration_thread;
+	struct list_head migration_queue;
+	int active_balance;
+	int cpu;
+} cpu_t;
+
+/*
  * This is the main, per-CPU runqueue data structure.
  *
  * Locking rule: those places that want to lock multiple runqueues
@@ -157,7 +200,6 @@ struct runqueue {
 	spinlock_t lock;
 	unsigned long nr_running, nr_switches, expired_timestamp,
 			nr_uninterruptible;
-	task_t *curr, *idle;
 	struct mm_struct *prev_mm;
 	prio_array_t *active, *expired, arrays[2];
 	int prev_cpu_load[NR_CPUS];
@@ -165,27 +207,39 @@ struct runqueue {
 	atomic_t *node_nr_running;
 	int prev_node_load[MAX_NUMNODES];
 #endif
-	task_t *migration_thread;
-	struct list_head migration_queue;
+	int nr_cpus;
+	cpu_t cpu[MAX_NR_SIBLINGS];
 
 	atomic_t nr_iowait;
 } ____cacheline_aligned;
 
 static struct runqueue runqueues[NR_CPUS] __cacheline_aligned;
 
-#define cpu_rq(cpu)		(runqueues + (cpu))
+#define cpu_rq(cpu)		(runqueues + (rq_idx(cpu)))
+#define cpu_int(c)		((cpu_rq(c))->cpu + cpu_idx(c))
+#define cpu_curr_ptr(cpu)	(cpu_int(cpu)->curr)
+#define cpu_idle_ptr(cpu)	(cpu_int(cpu)->idle)
+
 #define this_rq()		cpu_rq(smp_processor_id())
 #define task_rq(p)		cpu_rq(task_cpu(p))
-#define cpu_curr(cpu)		(cpu_rq(cpu)->curr)
 #define rt_task(p)		((p)->prio < MAX_RT_PRIO)
 
+#define migration_thread(cpu)	(cpu_int(cpu)->migration_thread)
+#define migration_queue(cpu)	(&cpu_int(cpu)->migration_queue)
+
+#if NR_CPUS > 1
+# define task_allowed(p, cpu)	((p)->cpus_allowed & (1UL << (cpu)))
+#else
+# define task_allowed(p, cpu)	1
+#endif
+
 /*
  * Default context-switch locking:
  */
 #ifndef prepare_arch_switch
 # define prepare_arch_switch(rq, next)	do { } while(0)
 # define finish_arch_switch(rq, next)	spin_unlock_irq(&(rq)->lock)
-# define task_running(rq, p)		((rq)->curr == (p))
+# define task_running(p)		(cpu_curr_ptr(task_cpu(p)) == (p))
 #endif
 
 #ifdef CONFIG_NUMA
@@ -414,8 +468,18 @@ static inline void resched_task(task_t *
 #endif
 }
 
+static inline void resched_cpu(int cpu)
+{
+	resched_task(cpu_curr_ptr(cpu));
+}
+
 #ifdef CONFIG_SMP
 
+static inline void set_task_cpu(struct task_struct *p, unsigned int cpu)
+{
+	p->thread_info->cpu = cpu;
+}
+
 /*
  * wait_task_inactive - wait for a thread to unschedule.
  *
@@ -433,7 +497,7 @@ void wait_task_inactive(task_t * p)
 repeat:
 	preempt_disable();
 	rq = task_rq(p);
-	if (unlikely(task_running(rq, p))) {
+	if (unlikely(task_running(p))) {
 		cpu_relax();
 		/*
 		 * enable/disable preemption just to make this
@@ -444,7 +508,7 @@ repeat:
 		goto repeat;
 	}
 	rq = task_rq_lock(p, &flags);
-	if (unlikely(task_running(rq, p))) {
+	if (unlikely(task_running(p))) {
 		task_rq_unlock(rq, &flags);
 		preempt_enable();
 		goto repeat;
@@ -452,6 +516,13 @@ repeat:
 	task_rq_unlock(rq, &flags);
 	preempt_enable();
 }
+
+#else
+
+static inline void set_task_cpu(struct task_struct *p, unsigned int cpu)
+{
+}
+
 #endif
 
 /*
@@ -466,10 +537,44 @@ repeat:
  */
 void kick_if_running(task_t * p)
 {
-	if ((task_running(task_rq(p), p)) && (task_cpu(p) != smp_processor_id()))
+	if ((task_running(p)) && (task_cpu(p) != smp_processor_id()))
 		resched_task(p);
 }
 
+static void wake_up_cpu(runqueue_t *rq, int cpu, task_t *p)
+{
+	cpu_t *curr_cpu;
+	task_t *curr;
+	int idx;
+
+	if (idle_cpu(cpu))
+		return resched_cpu(cpu);
+
+	for_each_sibling(idx, rq) {
+		curr_cpu = rq->cpu + idx;
+		if (!task_allowed(p, curr_cpu->cpu))
+			continue;
+		if (curr_cpu->idle == curr_cpu->curr)
+			return resched_cpu(curr_cpu->cpu);
+	}
+
+	if (p->prio < cpu_curr_ptr(cpu)->prio)
+		return resched_task(cpu_curr_ptr(cpu));
+	if (p->prio == cpu_curr_ptr(cpu)->prio &&
+			p->time_slice > cpu_curr_ptr(cpu)->time_slice)
+		return resched_task(cpu_curr_ptr(cpu));
+
+	for_each_sibling(idx, rq) {
+		curr_cpu = rq->cpu + idx;
+		if (!task_allowed(p, curr_cpu->cpu))
+			continue;
+		curr = curr_cpu->curr;
+		if (p->prio < curr->prio)
+			return resched_task(curr);
+		if (p->prio == curr->prio && p->time_slice > curr->time_slice)
+			return resched_task(curr);
+	}
+}
 /***
  * try_to_wake_up - wake up a thread
  * @p: the to-be-woken-up thread
@@ -491,6 +596,7 @@ static int try_to_wake_up(task_t * p, un
 	long old_state;
 	runqueue_t *rq;
 
+	BUG_ON(!p);
 repeat_lock_task:
 	rq = task_rq_lock(p, &flags);
 	old_state = p->state;
@@ -500,7 +606,7 @@ repeat_lock_task:
 			 * Fast-migrate the task if it's not running or runnable
 			 * currently. Do not violate hard affinity.
 			 */
-			if (unlikely(sync && !task_running(rq, p) &&
+			if (unlikely(sync && !task_running(p) &&
 				(task_cpu(p) != smp_processor_id()) &&
 				(p->cpus_allowed & (1UL << smp_processor_id())))) {
 
@@ -514,8 +620,7 @@ repeat_lock_task:
 				__activate_task(p, rq);
 			else {
 				requeue_waker = activate_task(p, rq);
-				if (p->prio < rq->curr->prio)
-					resched_task(rq->curr);
+				wake_up_cpu(rq, task_cpu(p), p);
 			}
 			success = 1;
 		}
@@ -692,8 +797,9 @@ unsigned long nr_running(void)
 	unsigned long i, sum = 0;
 
 	for (i = 0; i < NR_CPUS; i++)
-		sum += cpu_rq(i)->nr_running;
-
+		/* Shared runqueues are counted only once. */
+		if (!cpu_idx(i))
+			sum += cpu_rq(i)->nr_running;
 	return sum;
 }
 
@@ -704,7 +810,9 @@ unsigned long nr_uninterruptible(void)
 	for (i = 0; i < NR_CPUS; i++) {
 		if (!cpu_online(i))
 			continue;
-		sum += cpu_rq(i)->nr_uninterruptible;
+		/* Shared runqueues are counted only once. */
+		if (!cpu_idx(i))
+			sum += cpu_rq(i)->nr_uninterruptible;
 	}
 	return sum;
 }
@@ -863,7 +971,15 @@ static int find_busiest_node(int this_no
 
 #endif /* CONFIG_NUMA */
 
-#if CONFIG_SMP
+#if !CONFIG_SMP
+
+/*
+ * on UP we do not need to balance between CPUs:
+ */
+static inline void load_balance(runqueue_t *this_rq, int idle, unsigned long cpumask) { }
+static inline void rebalance_tick(runqueue_t *this_rq, int idle) { }
+
+#else
 
 /*
  * double_lock_balance - lock the busiest runqueue
@@ -979,17 +1095,7 @@ static inline void pull_task(runqueue_t 
 	set_task_cpu(p, this_cpu);
 	nr_running_inc(this_rq);
 	enqueue_task(p, this_rq->active);
-	/*
-	 * Note that idle threads have a prio of MAX_PRIO, for this test
-	 * to be always true for them.
-	 */
-	if (p->prio < this_rq->curr->prio)
-		set_need_resched();
-	else {
-		if (p->prio == this_rq->curr->prio &&
-				p->time_slice > this_rq->curr->time_slice)
-			set_need_resched();
-	}
+	wake_up_cpu(this_rq, this_cpu, p);
 }
 
 /*
@@ -1039,6 +1145,7 @@ skip_bitmap:
 		goto out_unlock;
 	}
 
+
 	head = array->queue + idx;
 	curr = head->prev;
 skip_queue:
@@ -1049,12 +1156,15 @@ skip_queue:
 	 * 1) running (obviously), or
 	 * 2) cannot be migrated to this CPU due to cpus_allowed, or
 	 * 3) are cache-hot on their current CPU.
+	 *
+	 * (except if we are in idle mode which is a more agressive
+	 *  form of rebalancing.)
 	 */
 
-#define CAN_MIGRATE_TASK(p,rq,this_cpu)					\
-	((jiffies - (p)->last_run > cache_decay_ticks) &&	\
-		!task_running(rq, p) &&					\
-			((p)->cpus_allowed & (1UL << (this_cpu))))
+#define CAN_MIGRATE_TASK(p,rq,cpu)		\
+	(((idle && AGRESSIVE_IDLE_STEAL) ||	\
+		(jiffies - (p)->last_run > cache_decay_ticks)) && \
+			!task_running(p) && task_allowed(p, cpu))
 
 	curr = curr->prev;
 
@@ -1077,6 +1187,133 @@ out:
 	;
 }
 
+#if CONFIG_SHARE_RUNQUEUE
+static void active_load_balance(runqueue_t *this_rq, int this_cpu)
+{
+	runqueue_t *rq;
+	int i, idx;
+
+	for (i = 0; i < NR_CPUS; i++) {
+		if (!cpu_online(i))
+			continue;
+		rq = cpu_rq(i);
+		if (rq == this_rq)
+			continue;
+ 		/*
+		 * If there's only one CPU mapped to this runqueue
+		 * then there can be no SMT imbalance:
+		 */
+		if (rq->nr_cpus == 1)
+			continue;
+		/*
+		 * Any SMT-specific imbalance?
+		 */
+		for_each_sibling(idx, rq)
+			if (rq->cpu[idx].idle == rq->cpu[idx].curr)
+				goto next_cpu;
+
+		/*
+		 * At this point it's sure that we have a SMT
+		 * imbalance: this (physical) CPU is idle but
+		 * another CPU has two (or more) tasks running.
+		 *
+		 * We wake up one of the migration threads (it
+		 * doesnt matter which one) and let it fix things up:
+		 */
+		if (!cpu_active_balance(i)) {
+			cpu_active_balance(i) = 1;
+			spin_unlock(&this_rq->lock);
+			if (rq->cpu[0].migration_thread)
+				wake_up_process(rq->cpu[0].migration_thread);
+			spin_lock(&this_rq->lock);
+		}
+next_cpu:
+	}
+}
+
+static void do_active_balance(runqueue_t *this_rq, int this_cpu)
+{
+	runqueue_t *rq;
+	int i, idx;
+
+	spin_unlock(&this_rq->lock);
+
+	cpu_active_balance(this_cpu) = 0;
+
+	/*
+	 * Is the imbalance still present?
+	 */
+	for_each_sibling(idx, this_rq)
+		if (this_rq->cpu[idx].idle == this_rq->cpu[idx].curr)
+			goto out;
+
+	for (i = 0; i < NR_CPUS; i++) {
+		if (!cpu_online(i))
+			continue;
+		rq = cpu_rq(i);
+		if (rq == this_rq)
+			continue;
+
+		/* completely idle CPU? */
+		if (rq->nr_running)
+			continue;
+
+		/*
+		 * At this point it's reasonably sure that we have an
+		 * imbalance. Since we are the migration thread, try to
+	 	 * balance a thread over to the target queue.
+		 */
+		spin_lock(&this_rq->lock);
+		this_rq->nr_running--;
+		spin_unlock(&this_rq->lock);
+
+		spin_lock(&rq->lock);
+		load_balance(rq, 1, cpu_to_node_mask(this_cpu));
+		spin_unlock(&rq->lock);
+
+		spin_lock(&this_rq->lock);
+		this_rq->nr_running++;
+		spin_unlock(&this_rq->lock);
+		goto out;
+	}
+out:
+	spin_lock(&this_rq->lock);
+}
+
+/*
+ * This routine is called to map a CPU into another CPU's runqueue.
+ *
+ * This must be called during bootup with the merged runqueue having
+ * no tasks.
+ */
+void sched_map_runqueue(int cpu1, int cpu2)
+{
+	runqueue_t *rq1 = cpu_rq(cpu1);
+	runqueue_t *rq2 = cpu_rq(cpu2);
+	int cpu2_idx_orig = cpu_idx(cpu2), cpu2_idx;
+
+	BUG_ON(rq1 == rq2 || rq2->nr_running || rq_idx(cpu1) != cpu1);
+	/*
+	 * At this point, we dont have anything in the runqueue yet. So,
+	 * there is no need to move processes between the runqueues.
+	 * Only, the idle processes should be combined and accessed
+	 * properly.
+	 */
+	cpu2_idx = rq1->nr_cpus++;
+
+	rq_idx(cpu2) = cpu1;
+	cpu_idx(cpu2) = cpu2_idx;
+	rq1->cpu[cpu2_idx].cpu = cpu2;
+	rq1->cpu[cpu2_idx].idle = rq2->cpu[cpu2_idx_orig].idle;
+	rq1->cpu[cpu2_idx].curr = rq2->cpu[cpu2_idx_orig].curr;
+	INIT_LIST_HEAD(&rq1->cpu[cpu2_idx].migration_queue);
+
+	/* just to be safe: */
+	rq2->cpu[cpu2_idx_orig].idle = NULL;
+	rq2->cpu[cpu2_idx_orig].curr = NULL;
+}
+#endif
+
 /*
  * One of the idle_cpu_tick() and busy_cpu_tick() functions will
  * get called every timer tick, on every CPU. Our balancing action
@@ -1145,15 +1382,9 @@ static void rebalance_tick(runqueue_t *t
 		spin_unlock(&this_rq->lock);
 	}
 }
-#else
-/*
- * on UP we do not need to balance between CPUs:
- */
-static inline void rebalance_tick(runqueue_t *this_rq, int idle)
-{
-}
 #endif
 
+
 DEFINE_PER_CPU(struct kernel_stat, kstat) = { { 0 } };
 
 /*
@@ -1181,12 +1412,13 @@ void scheduler_tick(int user_ticks, int 
 {
 	int cpu = smp_processor_id();
 	runqueue_t *rq = this_rq();
+	unsigned long j = jiffies;
 	task_t *p = current;
 
 	if (rcu_pending(cpu))
 		rcu_check_callbacks(cpu, user_ticks);
 
-	if (p == rq->idle) {
+	if (p == cpu_idle_ptr(cpu)) {
 		/* note: this timer irq context must be accounted for as well */
 		if (irq_count() - HARDIRQ_OFFSET >= SOFTIRQ_OFFSET)
 			kstat_cpu(cpu).cpustat.system += sys_ticks;
@@ -1209,6 +1441,7 @@ void scheduler_tick(int user_ticks, int 
 		return;
 	}
 	spin_lock(&rq->lock);
+  	/* Task might have expired already, but not scheduled off yet */
 	/*
 	 * The task was running during this tick - update the
 	 * time slice counter and the sleep average. Note: we
@@ -1244,7 +1477,7 @@ void scheduler_tick(int user_ticks, int 
 
 		if (!TASK_INTERACTIVE(p) || EXPIRED_STARVING(rq)) {
 			if (!rq->expired_timestamp)
-				rq->expired_timestamp = jiffies;
+				rq->expired_timestamp = j;
 			enqueue_task(p, rq->expired);
 		} else
 			enqueue_task(p, rq->active);
@@ -1261,11 +1494,11 @@ void scheduling_functions_start_here(voi
  */
 asmlinkage void schedule(void)
 {
+	int idx, this_cpu, retry = 0;
+	struct list_head *queue;
 	task_t *prev, *next;
-	runqueue_t *rq;
 	prio_array_t *array;
-	struct list_head *queue;
-	int idx;
+	runqueue_t *rq;
 
 	/*
 	 * Test if we are atomic.  Since do_exit() needs to call into
@@ -1283,6 +1516,7 @@ asmlinkage void schedule(void)
 need_resched:
 	preempt_disable();
 	prev = current;
+	this_cpu = smp_processor_id();
 	rq = this_rq();
 
 	release_kernel_lock(prev);
@@ -1307,14 +1541,17 @@ need_resched:
 	case TASK_RUNNING:
 		;
 	}
+
 pick_next_task:
 	if (unlikely(!rq->nr_running)) {
-#if CONFIG_SMP
 		load_balance(rq, 1, cpu_to_node_mask(smp_processor_id()));
 		if (rq->nr_running)
 			goto pick_next_task;
-#endif
-		next = rq->idle;
+		active_load_balance(rq, this_cpu);
+		if (rq->nr_running)
+			goto pick_next_task;
+pick_idle:
+		next = cpu_idle_ptr(this_cpu);
 		rq->expired_timestamp = 0;
 		goto switch_tasks;
 	}
@@ -1330,18 +1567,49 @@ pick_next_task:
 		rq->expired_timestamp = 0;
 	}
 
+new_array:
 	idx = sched_find_first_bit(array->bitmap);
-	queue = array->queue + idx;
-	next = list_entry(queue->next, task_t, run_list);
+  	queue = array->queue + idx;
+  	next = list_entry(queue->next, task_t, run_list);
+	if ((next != prev) && (rq_nr_cpus(rq) > 1)) {
+		struct list_head *tmp = queue->next;
+
+		while ((task_running(next) && (next != prev)) || !task_allowed(next, this_cpu)) {
+			tmp = tmp->next;
+			if (tmp != queue) {
+				next = list_entry(tmp, task_t, run_list);
+				continue;
+			}
+			idx = find_next_bit(array->bitmap, MAX_PRIO, ++idx);
+			if (idx == MAX_PRIO) {
+				if (retry || !rq->expired->nr_active) {
+					goto pick_idle;
+				}
+				/*
+				 * To avoid infinite changing of arrays,
+				 * when we have only tasks runnable by
+				 * sibling.
+				 */
+				retry = 1;
+
+				array = rq->expired;
+				goto new_array;
+			}
+			queue = array->queue + idx;
+			tmp = queue->next;
+			next = list_entry(tmp, task_t, run_list);
+		}
+	}
 
 switch_tasks:
 	prefetch(next);
 	clear_tsk_need_resched(prev);
-	RCU_qsctr(prev->thread_info->cpu)++;
+	RCU_qsctr(task_cpu(prev))++;
 
 	if (likely(prev != next)) {
 		rq->nr_switches++;
-		rq->curr = next;
+		cpu_curr_ptr(this_cpu) = next;
+		set_task_cpu(next, this_cpu);
 
 		prepare_arch_switch(rq, next);
 		prev = context_switch(rq, prev, next);
@@ -1604,9 +1872,8 @@ void set_user_nice(task_t *p, long nice)
 		 * If the task is running and lowered its priority,
 		 * or increased its priority then reschedule its CPU:
 		 */
-		if ((NICE_TO_PRIO(nice) < p->static_prio) ||
-							task_running(rq, p))
-			resched_task(rq->curr);
+		if ((NICE_TO_PRIO(nice) < p->static_prio) || task_running(p))
+			resched_task(cpu_curr_ptr(task_cpu(p)));
 	}
 out_unlock:
 	task_rq_unlock(rq, &flags);
@@ -1684,7 +1951,7 @@ int task_nice(task_t *p)
  */
 int task_curr(task_t *p)
 {
-	return cpu_curr(task_cpu(p)) == p;
+	return cpu_curr_ptr(task_cpu(p)) == p;
 }
 
 /**
@@ -1693,7 +1960,7 @@ int task_curr(task_t *p)
  */
 int idle_cpu(int cpu)
 {
-	return cpu_curr(cpu) == cpu_rq(cpu)->idle;
+	return cpu_curr_ptr(cpu) == cpu_idle_ptr(cpu);
 }
 
 /**
@@ -2243,7 +2510,7 @@ void __init init_idle(task_t *idle, int 
 	local_irq_save(flags);
 	double_rq_lock(idle_rq, rq);
 
-	idle_rq->curr = idle_rq->idle = idle;
+	cpu_curr_ptr(cpu) = cpu_idle_ptr(cpu) = idle;
 	deactivate_task(idle, rq);
 	idle->array = NULL;
 	idle->prio = MAX_PRIO;
@@ -2298,6 +2565,7 @@ void set_cpus_allowed(task_t *p, unsigne
 	unsigned long flags;
 	migration_req_t req;
 	runqueue_t *rq;
+	int cpu;
 
 #if 0 /* FIXME: Grab cpu_lock, return error on this case. --RR */
 	new_mask &= cpu_online_map;
@@ -2319,32 +2587,32 @@ void set_cpus_allowed(task_t *p, unsigne
 	 * If the task is not on a runqueue (and not running), then
 	 * it is sufficient to simply update the task's cpu field.
 	 */
-	if (!p->array && !task_running(rq, p)) {
+	if (!p->array && !task_running(p)) {
 		set_task_cpu(p, __ffs(p->cpus_allowed));
 		task_rq_unlock(rq, &flags);
 		return;
 	}
 	init_completion(&req.done);
 	req.task = p;
-	list_add(&req.list, &rq->migration_queue);
+	cpu = task_cpu(p);
+	list_add(&req.list, migration_queue(cpu));
 	task_rq_unlock(rq, &flags);
-
-	wake_up_process(rq->migration_thread);
+	wake_up_process(migration_thread(cpu));
 
 	wait_for_completion(&req.done);
 }
 
 /*
- * migration_thread - this is a highprio system thread that performs
+ * migration_task - this is a highprio system thread that performs
  * thread migration by 'pulling' threads into the target runqueue.
  */
-static int migration_thread(void * data)
+static int migration_task(void * data)
 {
 	/* Marking "param" __user is ok, since we do a set_fs(KERNEL_DS); */
 	struct sched_param __user param = { .sched_priority = MAX_RT_PRIO-1 };
 	int cpu = (long) data;
 	runqueue_t *rq;
-	int ret;
+	int ret, idx;
 
 	daemonize("migration/%d", cpu);
 	set_fs(KERNEL_DS);
@@ -2358,7 +2626,8 @@ static int migration_thread(void * data)
 	ret = setscheduler(0, SCHED_FIFO, &param);
 
 	rq = this_rq();
-	rq->migration_thread = current;
+	migration_thread(cpu) = current;
+	idx = cpu_idx(cpu);
 
 	for (;;) {
 		runqueue_t *rq_src, *rq_dest;
@@ -2369,8 +2638,10 @@ static int migration_thread(void * data)
 		task_t *p;
 
 		spin_lock_irqsave(&rq->lock, flags);
-		head = &rq->migration_queue;
-		current->state = TASK_INTERRUPTIBLE;
+		if (cpu_active_balance(cpu))
+			do_active_balance(rq, cpu);
+		head = migration_queue(cpu);
+		current->state = TASK_UNINTERRUPTIBLE;
 		if (list_empty(head)) {
 			spin_unlock_irqrestore(&rq->lock, flags);
 			schedule();
@@ -2399,8 +2670,7 @@ repeat:
 			if (p->array) {
 				deactivate_task(p, rq_src);
 				__activate_task(p, rq_dest);
-				if (p->prio < rq_dest->curr->prio)
-					resched_task(rq_dest->curr);
+				wake_up_cpu(rq_dest, cpu_dest, p);
 			}
 		}
 		double_rq_unlock(rq_src, rq_dest);
@@ -2418,12 +2688,13 @@ static int migration_call(struct notifie
 			  unsigned long action,
 			  void *hcpu)
 {
+	long cpu = (long) hcpu;
+
 	switch (action) {
 	case CPU_ONLINE:
-		printk("Starting migration thread for cpu %li\n",
-		       (long)hcpu);
-		kernel_thread(migration_thread, hcpu, CLONE_KERNEL);
-		while (!cpu_rq((long)hcpu)->migration_thread)
+		printk("Starting migration thread for cpu %li\n", cpu);
+		kernel_thread(migration_task, hcpu, CLONE_KERNEL);
+		while (!migration_thread(cpu))
 			yield();
 		break;
 	}
@@ -2488,6 +2759,7 @@ __init static void init_kstat(void) {
 	register_cpu_notifier(&kstat_nb);
 }
 
+
 void __init sched_init(void)
 {
 	runqueue_t *rq;
@@ -2498,11 +2770,20 @@ void __init sched_init(void)
 	for (i = 0; i < NR_CPUS; i++) {
 		prio_array_t *array;
 
+		/*
+		 * Start with a 1:1 mapping between CPUs and runqueues:
+		 */
+#if CONFIG_SHARE_RUNQUEUE
+		rq_idx(i) = i;
+		cpu_idx(i) = 0;
+#endif
 		rq = cpu_rq(i);
 		rq->active = rq->arrays;
 		rq->expired = rq->arrays + 1;
 		spin_lock_init(&rq->lock);
-		INIT_LIST_HEAD(&rq->migration_queue);
+		INIT_LIST_HEAD(migration_queue(i));
+		rq->nr_cpus = 1;
+		rq->cpu[cpu_idx(i)].cpu = i;
 		atomic_set(&rq->nr_iowait, 0);
 		nr_running_init(rq);
 
@@ -2520,9 +2801,9 @@ void __init sched_init(void)
 	 * We have to do a little magic to get the first
 	 * thread right in SMP mode.
 	 */
-	rq = this_rq();
-	rq->curr = current;
-	rq->idle = current;
+	cpu_curr_ptr(smp_processor_id()) = current;
+	cpu_idle_ptr(smp_processor_id()) = current;
+
 	set_task_cpu(current, smp_processor_id());
 	wake_up_forked_process(current);
 
--- linux/init/main.c.orig	
+++ linux/init/main.c	
@@ -367,7 +367,14 @@ static void __init smp_init(void)
 
 static void rest_init(void)
 {
+	/* 
+	 *	We count on the initial thread going ok 
+	 *	Like idlers init is an unlocked kernel thread, which will
+	 *	make syscalls (and thus be locked).
+	 */
+	init_idle(current, smp_processor_id());
 	kernel_thread(init, NULL, CLONE_KERNEL);
+
 	unlock_kernel();
  	cpu_idle();
 } 
@@ -453,13 +460,6 @@ asmlinkage void __init start_kernel(void
 	check_bugs();
 	printk("POSIX conformance testing by UNIFIX\n");
 
-	/* 
-	 *	We count on the initial thread going ok 
-	 *	Like idlers init is an unlocked kernel thread, which will
-	 *	make syscalls (and thus be locked).
-	 */
-	init_idle(current, smp_processor_id());
-
 	/* Do the rest non-__init'ed, we're now alive */
 	rest_init();
 }

Re: [patch] HT scheduler, sched-2.5.68-A9

[Dave Jones]

On Mon, Apr 21, 2003 at 03:13:31PM -0400, Ingo Molnar wrote:

 > +/*
 > + * Is there a way to do this via Kconfig?
 > + */
 > +#if CONFIG_NR_SIBLINGS_2
 > +# define CONFIG_NR_SIBLINGS 2
 > +#elif CONFIG_NR_SIBLINGS_4
 > +# define CONFIG_NR_SIBLINGS 4
 > +#else
 > +# define CONFIG_NR_SIBLINGS 0
 > +#endif
 > +

Maybe this would be better resolved at runtime ?
With the above patch, you'd need three seperate kernel images
to run optimally on a system in each of the cases.
The 'vendor kernel' scenario here looks ugly to me.

 > +#if CONFIG_NR_SIBLINGS
 > +# define CONFIG_SHARE_RUNQUEUE 1
 > +#else
 > +# define CONFIG_SHARE_RUNQUEUE 0
 > +#endif

And why can't this just be a

	if (ht_enabled)
		shared_runqueue = 1;

Dumping all this into the config system seems to be the
wrong direction IMHO. The myriad of runtime knobs in the
scheduler already is bad enough, without introducing
compile time ones as well.

		Dave

Re: [patch] HT scheduler, sched-2.5.68-A9

[Ingo Molnar]

On Tue, 22 Apr 2003, Dave Jones wrote:

> Maybe this would be better resolved at runtime ? With the above patch,
> you'd need three seperate kernel images to run optimally on a system in
> each of the cases. The 'vendor kernel' scenario here looks ugly to me.

it's not a problem - vendors enable it and that's all. But the majority of 
SMP systems does not need a shared runqueue, so the associated overhead 
(which, while small, is nonzero) can be avoided.

> Dumping all this into the config system seems to be the wrong direction
> IMHO. The myriad of runtime knobs in the scheduler already is bad
> enough, without introducing compile time ones as well.

what runtime knobs? I've avoided as many of them as possible.

	Ingo

Re: [patch] HT scheduler, sched-2.5.68-A9

[Rick Lindsley]

Ingo, several questions.

What makes this statement:

    * At this point it's sure that we have a SMT
    * imbalance: this (physical) CPU is idle but
    * another CPU has two (or more) tasks running.

true?  Do you mean "this cpu/sibling set are all idle but another
cpu/sibling set are all non-idle"?  Are we assuming that because both
a physical processor and its sibling are not idle, that it is better to
move a task from the sibling to a physical processor?  In other words,
we are presuming that the case where the task on the physical processor
and the task(s) on the sibling(s) are actually benefitting from the
relationship is rare?

    * We wake up one of the migration threads (it
    * doesnt matter which one) and let it fix things up:

So although a migration thread normally pulls tasks to it, we've altered
migration_thread now so that when cpu_active_balance() is set for its
cpu, it will go find a cpu/sibling set in which all siblings are busy
(knowing it has a good chance of finding one), decrement nr_running in
the runqueue it has found, call load_balance() on the queue which is
idle, and hope that load_balance will again find the busy queue (the
same queue as the migration thread's) and decide to move a task over?

whew. So why are we perverting the migration thread to push rather
than pull?  If active_load_balance() finds a imbalance, why must we use
such indirection?  Why decrement nr_running?  Couldn't we put together
a migration_req_t for the target queue's migration thread?

Making the migration thread TASK_UNINTERRUPTIBLE has the nasty side
effect of artificially raising the load average.  Why is this changed?

Rick

Re: [patch] HT scheduler, sched-2.5.68-A9

[Ingo Molnar]

On Tue, 22 Apr 2003, Rick Lindsley wrote:

> Ingo, several questions.
> 
> What makes this statement:
> 
>     * At this point it's sure that we have a SMT
>     * imbalance: this (physical) CPU is idle but
>     * another CPU has two (or more) tasks running.
> 
> true?  Do you mean "this cpu/sibling set are all idle but another
> cpu/sibling set are all non-idle"? [...]

yes, precisely.

> [...] Are we assuming that because both a physical processor and its
> sibling are not idle, that it is better to move a task from the sibling
> to a physical processor?  In other words, we are presuming that the case
> where the task on the physical processor and the task(s) on the
> sibling(s) are actually benefitting from the relationship is rare?

yes. This 'un-sharing' of contexts happens unconditionally, whenever we
notice the situation. (ie. whenever a CPU goes completely idle and notices
an overloaded physical CPU.) On the HT system i have i have measure this
to be a beneficial move even for the most trivial things like infinite
loop-counting.

the more per-logical-CPU cache a given SMT implementation has, the less
beneficial this move becomes - in that case the system should rather be
set up as a NUMA topology and scheduled via the NUMA scheduler.

>     * We wake up one of the migration threads (it
>     * doesnt matter which one) and let it fix things up:
> 
> So although a migration thread normally pulls tasks to it, we've altered
> migration_thread now so that when cpu_active_balance() is set for its
> cpu, it will go find a cpu/sibling set in which all siblings are busy
> (knowing it has a good chance of finding one), decrement nr_running in
> the runqueue it has found, call load_balance() on the queue which is
> idle, and hope that load_balance will again find the busy queue (the
> same queue as the migration thread's) and decide to move a task over?

yes.

> whew. So why are we perverting the migration thread to push rather than
> pull? If active_load_balance() finds a imbalance, why must we use such
> indirection?  Why decrement nr_running?  Couldn't we put together a
> migration_req_t for the target queue's migration thread?

i'm not sure what you mean by perverting the migration thread to push
rather to pull, as migration threads always push - it's not different in
this case either. Since the task in question is running in an
un-cooperative way at the moment of active-balancing, that CPU needs to
run the high-prio migration thread, which pushes the task to the proper
CPU after that point. [if the push is still necessary.]

we could use a migration_req_t for this, in theory, but active balancing
is independent of ->cpus_allowed, so some special code would still be
needed. Also, active balancing is non-queued by nature. Is there a big
difference?

> Making the migration thread TASK_UNINTERRUPTIBLE has the nasty side
> effect of artificially raising the load average.  Why is this changed?

agreed, this is an oversight, i fixed it in my tree.

	Ingo

Re: [patch] HT scheduler, sched-2.5.68-A9

[Rick Lindsley]

Ingo, thanks for clearing up some things.

    yes. This 'un-sharing' of contexts happens unconditionally, whenever
    we notice the situation. (ie. whenever a CPU goes completely idle
    and notices an overloaded physical CPU.) On the HT system i have i
    have measure this to be a beneficial move even for the most trivial
    things like infinite loop-counting.

I have access to a 4-proc HT so I can try it there too. Did you test with
micro-benchmarks like the loop-counting or did you use something bigger?

    the more per-logical-CPU cache a given SMT implementation has,
    the less beneficial this move becomes - in that case the system
    should rather be set up as a NUMA topology and scheduled via the
    NUMA scheduler.

	whew. So why are we perverting the migration thread to push
	rather than pull? If active_load_balance() finds a imbalance,
	why must we use such indirection?  Why decrement nr_running?
	Couldn't we put together a migration_req_t for the target queue's
	migration thread?

    i'm not sure what you mean by perverting the migration thread to
    push rather to pull, as migration threads always push - it's not
    different in this case either.

My bad -- I read the comments around migration_thread(), and they could
probably be improved.  When I looked at the code, yes, it's more of
a push.  The migration thread process occupies the processor so that
you can be sure the process-of-interest is not running and can be more
easily manipulated.

    we could use a migration_req_t for this, in theory, but active
    balancing is independent of ->cpus_allowed, so some special code
    would still be needed.

I'm just looking for the cleanest approach.  Functionally I see no
difference; just seems like we go running through the queues several times
(not all in active_load_balance) before active_load_balance has achieved
its goal.  I was thinking maybe a directed move by the migration thread
("move THAT process to THAT processor/runqueue"), similar to what is done
in set_cpus_allowed(), might be cleaner and faster.  Maybe I'll try that.

    Also, active balancing is non-queued by nature. Is there a big
    difference?

I'm not sure active balancing really is independent of cpus_allowed.
Yes, all the searches are done without that restriction in place, but
then we ultimately call load_balance(), which *will* care. load_balance()
may end up not moving what we wanted (or anything at all.)

Rick

Re: [patch] HT scheduler, sched-2.5.68-A9

[Ingo Molnar]

On Tue, 22 Apr 2003, Rick Lindsley wrote:

>     yes. This 'un-sharing' of contexts happens unconditionally, whenever
>     we notice the situation. (ie. whenever a CPU goes completely idle
>     and notices an overloaded physical CPU.) On the HT system i have i
>     have measure this to be a beneficial move even for the most trivial
>     things like infinite loop-counting.
> 
> I have access to a 4-proc HT so I can try it there too. Did you test
> with micro-benchmarks like the loop-counting or did you use something
> bigger?

it is very obviously the case that two tasks running on different physical
CPUs outperform two tasks running on the same physical CPU. I have
attempted to find the best-case sharing - and even that one underperforms.  
I measured cache-heavy gcc compilation as well, there there was almost no
speedup due to HT.

but it's not really a problem - the logical CPUs are probably quite cheap
on the physical side, so any improvement in overload situations is a help.  
But once the overload situation stops, we should avoid the false sharing
and balance over those tasks to one physical CPU each.

in any case, please feel free to do measurements in this direction
nevertheless, more numbers never hurt.

>     the more per-logical-CPU cache a given SMT implementation has,
>     the less beneficial this move becomes - in that case the system
>     should rather be set up as a NUMA topology and scheduled via the
>     NUMA scheduler.
> 
> 	whew. So why are we perverting the migration thread to push
> 	rather than pull? If active_load_balance() finds a imbalance,
> 	why must we use such indirection?  Why decrement nr_running?
> 	Couldn't we put together a migration_req_t for the target queue's
> 	migration thread?
> 
>     i'm not sure what you mean by perverting the migration thread to
>     push rather to pull, as migration threads always push - it's not
>     different in this case either.
> 
> My bad -- I read the comments around migration_thread(), and they could
> probably be improved. [...]

i'll fix the comments up. And the migration concept originally was a pull
thing, but now it has arrived to a clean push model.

> [...] When I looked at the code, yes, it's more of a push.  The
> migration thread process occupies the processor so that you can be sure
> the process-of-interest is not running and can be more easily
> manipulated.

yes, that's the core idea.

>     Also, active balancing is non-queued by nature. Is there a big
>     difference?
> 
> I'm not sure active balancing really is independent of cpus_allowed.

of course we never balance to a CPU not allowed, but what i meant is that
the forced migration triggered by a ->cpus_allowed change [ie. the removal
of the current CPU from the process' allowed CPU mask] is conceptually
different from the forced migration of a task between two allowed CPUs.

> Yes, all the searches are done without that restriction in place, but
> then we ultimately call load_balance(), which *will* care.
> load_balance() may end up not moving what we wanted (or anything at
> all.)

load_balance() will most definitly balance the task in question, in the
active-balance case. The only reason why it didnt succeed earlier is
because load_balance() is a passive "pull" concept, so it is not able to
break up the false sharing between those two tasks that are both actively
running. [it correctly sees a 2:0 imbalance between the runqueues and
tries to balance them, but both tasks are running.] This is why the "push"  
concept of active-balancing has to kick in.

in fact in the active-balance case the imbalance is 3:0, because the
migration thread is running too, so we decrease nr_running artifically,
before calling load_balance(). Otherwise a 3:1 setup could cause a false
migration. [the real load situation is 2:1 in that case.]

we dont keep the runqueues locked while these migration requests are
pending, so there's a small window for the balancing to get behind - but
that risk is present with any statistical approach anyway.

	Ingo

Re: several messages

[Bill Davidsen]

On Tue, 22 Apr 2003, Dave Jones wrote:

> On Mon, Apr 21, 2003 at 03:13:31PM -0400, Ingo Molnar wrote:
> 
>  > +/*
>  > + * Is there a way to do this via Kconfig?
>  > + */
>  > +#if CONFIG_NR_SIBLINGS_2
>  > +# define CONFIG_NR_SIBLINGS 2
>  > +#elif CONFIG_NR_SIBLINGS_4
>  > +# define CONFIG_NR_SIBLINGS 4
>  > +#else
>  > +# define CONFIG_NR_SIBLINGS 0
>  > +#endif
>  > +
> 
> Maybe this would be better resolved at runtime ?
> With the above patch, you'd need three seperate kernel images
> to run optimally on a system in each of the cases.
> The 'vendor kernel' scenario here looks ugly to me.
> 
>  > +#if CONFIG_NR_SIBLINGS
>  > +# define CONFIG_SHARE_RUNQUEUE 1
>  > +#else
>  > +# define CONFIG_SHARE_RUNQUEUE 0
>  > +#endif
> 
> And why can't this just be a
> 
> 	if (ht_enabled)
> 		shared_runqueue = 1;
> 
> Dumping all this into the config system seems to be the
> wrong direction IMHO. The myriad of runtime knobs in the
> scheduler already is bad enough, without introducing
> compile time ones as well.

May I add my "I don't understand this, either" at this point? It seems
desirable to have this particular value determined at runtime. 

On Tue, 22 Apr 2003, Ingo Molnar wrote:

> 
> On Tue, 22 Apr 2003, Rick Lindsley wrote:

> > [...] Are we assuming that because both a physical processor and its
> > sibling are not idle, that it is better to move a task from the sibling
> > to a physical processor?  In other words, we are presuming that the case
> > where the task on the physical processor and the task(s) on the
> > sibling(s) are actually benefitting from the relationship is rare?
> 
> yes. This 'un-sharing' of contexts happens unconditionally, whenever we
> notice the situation. (ie. whenever a CPU goes completely idle and notices
> an overloaded physical CPU.) On the HT system i have i have measure this
> to be a beneficial move even for the most trivial things like infinite
> loop-counting.
> 
> the more per-logical-CPU cache a given SMT implementation has, the less
> beneficial this move becomes - in that case the system should rather be
> set up as a NUMA topology and scheduled via the NUMA scheduler.

Have you done any tests with a threaded process running on a single CPU in
the siblings? If they are sharing data and locks in the same cache it's
not obvious (to me at least) that it would be faster in two CPUs having to
do updates. That's a question, not an implication that it is significantly
better in just one, a threaded program with only two threads is not as
likely to be doing the same thing in both, perhaps.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: several messages

[Rick Lindsley]

    Have you done any tests with a threaded process running on a single CPU in
    the siblings? If they are sharing data and locks in the same cache it's
    not obvious (to me at least) that it would be faster in two CPUs having to
    do updates. That's a question, not an implication that it is significantly
    better in just one, a threaded program with only two threads is not as
    likely to be doing the same thing in both, perhaps.

True.  I have a hunch (and it's only a hunch -- no hard data!) that
two threads that are sharing the same data will do better if they can
be located on a physical/sibling processor group.  For workloads where
you really do have two distinct processes, or even threads but which are
operating on wholly different portions of data or code, moving them to
separate physical processors may be warranted.  The key is whether the
work of one sibling is destroying the cache of another.

Rick

Re: several messages

[Ingo Molnar]

On Tue, 22 Apr 2003, Rick Lindsley wrote:

> True.  I have a hunch (and it's only a hunch -- no hard data!) that two
> threads that are sharing the same data will do better if they can be
> located on a physical/sibling processor group.  For workloads where you
> really do have two distinct processes, or even threads but which are
> operating on wholly different portions of data or code, moving them to
> separate physical processors may be warranted.  The key is whether the
> work of one sibling is destroying the cache of another.

If two threads have a workload that wants to be co-scheduled then the SMP
scheduler will do damage to them anyway - independently of any HT
scheduling decisions. One solution for such specific cases is to use the
CPU-binding API to move those threads to the same physical CPU. If there's
some common class of applications where this is the common case, then we
could start thinking about automatic support for them.

	Ingo

Re: [patch] HT scheduler, sched-2.5.68-A9

[Martin J. Bligh]

> the attached patch (against 2.5.68 or BK-curr) is the latest
> implementation of the "no compromises" HT-scheduler. I fixed a couple of
> bugs, and the scheduler is now stable and behaves properly on a
> 2-CPU-2-sibling HT testbox. The patch can also be downloaded from:
> 
> 	http://redhat.com/~mingo/O(1)-scheduler/
> 
> bug reports, suggestions welcome,

Hmmm. When the machine is loaded up fully, this seems OK, but for lower
loads, it seems to have a significant degredation. This is on my normal
16-way machine, no HT at all ... just checking for degredations. At a
guess, maybe it's bouncing stuff around between runqueues under low
loads?

Are there things embedded in here that change stuff for non-HT machines?

M.

DISCLAIMER: SPEC(tm) and the benchmark name SDET(tm) are registered
trademarks of the Standard Performance Evaluation Corporation. This 
benchmarking was performed for research purposes only, and the run results
are non-compliant and not-comparable with any published results.

Results are shown as percentages of the first set displayed

SDET 1  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         0.7%
                2.5.68-ht        72.9%         1.8%

SDET 2  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         2.8%
                2.5.68-ht        73.7%         2.1%

SDET 4  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         1.0%
                2.5.68-ht        62.5%        47.0%

SDET 8  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         0.6%
                2.5.68-ht        92.6%         1.0%

SDET 16  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         0.1%
                2.5.68-ht       100.0%         0.5%

SDET 32  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         0.4%
                2.5.68-ht        99.1%         0.6%

SDET 64  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         0.2%
                2.5.68-ht        99.0%         0.1%

SDET 128  (see disclaimer)
                           Throughput    Std. Dev
                   2.5.68       100.0%         0.1%
                2.5.68-ht        99.0%         0.1%

diffprofile for SDET 4:

     12339    28.7% total
     10551    27.0% default_idle
       313    75.6% page_add_rmap
       240   110.1% copy_page_range
       192   243.0% do_wp_page
       174    31.1% page_remove_rmap
        51    20.6% zap_pte_range
        48    65.8% do_anonymous_page
        48    41.0% find_get_page
        48   160.0% copy_mm
        41    41.4% __d_lookup
        38    44.7% __copy_to_user_ll
        34    82.9% __copy_user_intel
        32   110.3% copy_process
        28    28.9% release_pages
        27   158.8% current_kernel_time
        23   121.1% release_task
        23    35.4% kmem_cache_free
        20   125.0% free_hot_cold_page
        17    77.3% buffered_rmqueue
        15   100.0% __copy_from_user_ll
        15    24.2% do_no_page
        13    16.9% do_page_fault
        12    44.4% pte_alloc_one
        12    57.1% schedule
        12   400.0% copy_files
        11    73.3% exit_notify
        10    29.4% path_lookup
         9   180.0% dup_task_struct
         9    27.3% atomic_dec_and_lock
         7    31.8% handle_mm_fault
         7   140.0% block_invalidatepage
         7    38.9% __pte_chain_free
         7    14.0% clear_page_tables
         7   116.7% fd_install
         7    77.8% write_profile
         7   350.0% generic_delete_inode
         6    54.5% ext2_update_inode
         6    60.0% do_page_cache_readahead
         6    66.7% dentry_open

Which looks like maybe more off-node NUMA accesses in there ...
is there some new rebalance mechanism that could be bouncing
stuff across nodes?

Re: [patch] HT scheduler, sched-2.5.68-A9

[Ingo Molnar]

On Tue, 22 Apr 2003, Martin J. Bligh wrote:

> Hmmm. When the machine is loaded up fully, this seems OK, but for lower
> loads, it seems to have a significant degredation. This is on my normal
> 16-way machine, no HT at all ... just checking for degredations. At a
> guess, maybe it's bouncing stuff around between runqueues under low
> loads?

hm - it should not cause any change in the non-HT case. Oh, there's one
thing that could make a difference on SMP - could you change
AGRESSIVE_IDLE_STEAL back to 0?

> Are there things embedded in here that change stuff for non-HT machines?

not intentionally, but apparently there's some sort of effect. The idle
steal thing could explain it.

	Ingo

[PATCH v2 0/3] x86/mm: INVPCID support

[Andy Lutomirski]

Ingo, before applying this, please apply these two KASAN fixes:

http://lkml.kernel.org/g/1452516679-32040-2-git-send-email-aryabinin@virtuozzo.com
http://lkml.kernel.org/g/1452516679-32040-3-git-send-email-aryabinin@virtuozzo.com

Without those fixes, this series will trigger a KASAN bug.

This is a straightforward speedup on Ivy Bridge and newer, IIRC.
(I tested on Skylake.  INVPCID is not available on Sandy Bridge.
I don't have Ivy Bridge, Haswell or Broadwell to test on, so I
could be wrong as to when the feature was introduced.)

I think we should consider these patches separately from the rest
of the PCID stuff -- they barely interact, and this part is much
simpler and is useful on its own.

This is exactly identical to patches 2-4 of the PCID RFC series.

Andy Lutomirski (3):
  x86/mm: Add INVPCID helpers
  x86/mm: Add a noinvpcid option to turn off INVPCID
  x86/mm: If INVPCID is available, use it to flush global mappings

 Documentation/kernel-parameters.txt |  2 ++
 arch/x86/include/asm/tlbflush.h     | 50 +++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/common.c        | 16 ++++++++++++
 3 files changed, 68 insertions(+)

-- 
2.5.0

Re: [PATCH v2 0/3] x86/mm: INVPCID support

[Ingo Molnar]

* Andy Lutomirski <luto@kernel.org> wrote:

> Ingo, before applying this, please apply these two KASAN fixes:
> 
> http://lkml.kernel.org/g/1452516679-32040-2-git-send-email-aryabinin@virtuozzo.com
> http://lkml.kernel.org/g/1452516679-32040-3-git-send-email-aryabinin@virtuozzo.com
> 
> Without those fixes, this series will trigger a KASAN bug.
> 
> This is a straightforward speedup on Ivy Bridge and newer, IIRC.
> (I tested on Skylake.  INVPCID is not available on Sandy Bridge.
> I don't have Ivy Bridge, Haswell or Broadwell to test on, so I
> could be wrong as to when the feature was introduced.)
> 
> I think we should consider these patches separately from the rest
> of the PCID stuff -- they barely interact, and this part is much
> simpler and is useful on its own.
> 
> This is exactly identical to patches 2-4 of the PCID RFC series.
> 
> Andy Lutomirski (3):
>   x86/mm: Add INVPCID helpers
>   x86/mm: Add a noinvpcid option to turn off INVPCID
>   x86/mm: If INVPCID is available, use it to flush global mappings
> 
>  Documentation/kernel-parameters.txt |  2 ++
>  arch/x86/include/asm/tlbflush.h     | 50 +++++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/common.c        | 16 ++++++++++++
>  3 files changed, 68 insertions(+)

Ok, I'll pick these up tomorrow unless there are objections.

Thanks,

	Ingo

Re: several messages

[Thomas Gleixner]

On Mon, 25 Jan 2016, Andy Lutomirski wrote:
> This is a straightforward speedup on Ivy Bridge and newer, IIRC.
> (I tested on Skylake.  INVPCID is not available on Sandy Bridge.
> I don't have Ivy Bridge, Haswell or Broadwell to test on, so I
> could be wrong as to when the feature was introduced.)

Haswell and Broadwell have it. No idea about ivy bridge.
 
Thanks,

	tglx

Re: several messages

[Borislav Petkov]

On Wed, Jan 27, 2016 at 11:09:04AM +0100, Thomas Gleixner wrote:
> On Mon, 25 Jan 2016, Andy Lutomirski wrote:
> > This is a straightforward speedup on Ivy Bridge and newer, IIRC.
> > (I tested on Skylake.  INVPCID is not available on Sandy Bridge.
> > I don't have Ivy Bridge, Haswell or Broadwell to test on, so I
> > could be wrong as to when the feature was introduced.)
>
> Haswell and Broadwell have it. No idea about ivy bridge.

I have an IVB model 58. It doesn't have it:

CPUID_0x00000007: EAX=0x00000000, EBX=0x00000281, ECX=0x00000000, EDX=0x00000000

INVPCID should be EBX[10].

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[PATCH 00/13] Add VT-d Posted-Interrupts support for KVM

[Feng Wu]

VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
With VT-d Posted-Interrupts enabled, external interrupts from
direct-assigned devices can be delivered to guests without VMM
intervention when guest is running in non-root mode.

You can find the VT-d Posted-Interrtups Spec. in the following URL:
http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/vt-directed-io-spec.html

Feng Wu (13):
  iommu/vt-d: VT-d Posted-Interrupts feature detection
  KVM: Initialize VT-d Posted-Interrtups Descriptor
  KVM: Add KVM_CAP_PI to detect VT-d Posted-Interrtups
  iommu/vt-d: Adjust 'struct irte' to better suit for VT-d
    Posted-Interrupts
  KVM: Update IRTE according to guest interrupt configuration changes
  KVM: Add some helper functions for Posted-Interrupts
  x86, irq: Define a global vector for VT-d Posted-Interrupts
  KVM: Update Posted-Interrupts descriptor during VCPU scheduling
  KVM: Change NDST field after VCPU scheduling
  KVM: Add the handler for Wake-up Vector
  KVM: Suppress posted-interrupt when 'SN' is set
  iommu/vt-d: No need to migrating irq for VT-d Posted-Interrtups
  iommu/vt-d: Add a command line parameter for VT-d posted-interrupts

 arch/x86/include/asm/entry_arch.h    |    2 +
 arch/x86/include/asm/hardirq.h       |    1 +
 arch/x86/include/asm/hw_irq.h        |    2 +
 arch/x86/include/asm/irq_remapping.h |    7 +
 arch/x86/include/asm/irq_vectors.h   |    1 +
 arch/x86/include/asm/kvm_host.h      |    9 ++
 arch/x86/kernel/apic/apic.c          |    1 +
 arch/x86/kernel/entry_64.S           |    2 +
 arch/x86/kernel/irq.c                |   27 ++++
 arch/x86/kernel/irqinit.c            |    2 +
 arch/x86/kvm/vmx.c                   |  257 +++++++++++++++++++++++++++++++++-
 arch/x86/kvm/x86.c                   |   53 ++++++-
 drivers/iommu/amd_iommu.c            |    6 +
 drivers/iommu/intel_irq_remapping.c  |   83 +++++++++--
 drivers/iommu/irq_remapping.c        |   20 +++
 drivers/iommu/irq_remapping.h        |    8 +
 include/linux/dmar.h                 |   30 ++++-
 include/linux/intel-iommu.h          |    1 +
 include/linux/kvm_host.h             |   25 ++++
 include/uapi/linux/kvm.h             |    2 +
 virt/kvm/assigned-dev.c              |  141 +++++++++++++++++++
 virt/kvm/irq_comm.c                  |    4 +-
 virt/kvm/irqchip.c                   |   11 --
 virt/kvm/kvm_main.c                  |   14 ++
 24 files changed, 667 insertions(+), 42 deletions(-)

[PATCH 13/13] iommu/vt-d: Add a command line parameter for VT-d posted-interrupts

[Feng Wu]

Enable VT-d Posted-Interrtups and add a command line
parameter for it.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 drivers/iommu/irq_remapping.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 0e36860..3cb9429 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -23,7 +23,7 @@ int irq_remap_broken;
 int disable_sourceid_checking;
 int no_x2apic_optout;
 
-int disable_irq_post = 1;
+int disable_irq_post = 0;
 int irq_post_enabled = 0;
 EXPORT_SYMBOL_GPL(irq_post_enabled);
 
@@ -206,6 +206,13 @@ static __init int setup_irqremap(char *str)
 }
 early_param("intremap", setup_irqremap);
 
+static __init int setup_nointpost(char *str)
+{
+	disable_irq_post = 1;
+	return 0;
+}
+early_param("nointpost", setup_nointpost);
+
 void __init setup_irq_remapping_ops(void)
 {
 	remap_ops = &intel_irq_remap_ops;
-- 
1.7.1

Re: several messages

[Thomas Gleixner]

On Mon, 10 Nov 2014, Feng Wu wrote:

> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
> With VT-d Posted-Interrupts enabled, external interrupts from
> direct-assigned devices can be delivered to guests without VMM
> intervention when guest is running in non-root mode.

Can you please talk to Jiang and synchronize your work with his
refactoring of the x86 interrupt handling subsystem.

I want this stuff cleaned up first before we add new stuff to it.

Thanks,

	tglx

Re: several messages

[Jiang Liu]

On 2014/11/11 2:15, Thomas Gleixner wrote:
> On Mon, 10 Nov 2014, Feng Wu wrote:
> 
>> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
>> With VT-d Posted-Interrupts enabled, external interrupts from
>> direct-assigned devices can be delivered to guests without VMM
>> intervention when guest is running in non-root mode.
> 
> Can you please talk to Jiang and synchronize your work with his
> refactoring of the x86 interrupt handling subsystem.
> 
> I want this stuff cleaned up first before we add new stuff to it.
Hi Thomas,
	Just talked with Feng, we will focused on refactor first and
then add posted interrupt support.
Regards!
Gerry

> 
> Thanks,
> 
> 	tglx
> 

RE: several messages

[Wu, Feng]

> -----Original Message-----
> From: Jiang Liu [mailto:jiang.liu@linux.intel.com]
> Sent: Tuesday, November 11, 2014 10:29 AM
> To: Thomas Gleixner; Wu, Feng
> Cc: gleb@kernel.org; pbonzini@redhat.com; David Woodhouse;
> joro@8bytes.org; mingo@redhat.com; H. Peter Anvin; x86@kernel.org;
> kvm@vger.kernel.org; iommu@lists.linux-foundation.org; LKML
> Subject: Re: several messages
> 
> On 2014/11/11 2:15, Thomas Gleixner wrote:
> > On Mon, 10 Nov 2014, Feng Wu wrote:
> >
> >> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
> >> With VT-d Posted-Interrupts enabled, external interrupts from
> >> direct-assigned devices can be delivered to guests without VMM
> >> intervention when guest is running in non-root mode.
> >
> > Can you please talk to Jiang and synchronize your work with his
> > refactoring of the x86 interrupt handling subsystem.
> >
> > I want this stuff cleaned up first before we add new stuff to it.
> Hi Thomas,
> 	Just talked with Feng, we will focused on refactor first and
> then add posted interrupt support.
> Regards!
> Gerry

No problem!

Thanks,
Feng

> 
> >
> > Thanks,
> >
> > 	tglx
> >

[PATCHv2] netfilter: add CHECKSUM target

[Michael S. Tsirkin]

This adds a `CHECKSUM' target, which can be used in the iptables mangle
table.

You can use this target to compute and fill in the checksum in
a packet that lacks a checksum.  This is particularly useful,
if you need to work around old applications such as dhcp clients,
that do not work well with checksum offloads, but don't want to
disable checksum offload in your device.

The problem happens in the field with virtualized applications.
For reference, see Red Hat bz 605555, as well as
http://www.spinics.net/lists/kvm/msg37660.html

Typical expected use (helps old dhclient binary running in a VM):
iptables -A POSTROUTING -t mangle -p udp --dport bootpc \
	-j CHECKSUM --checksum-fill

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Changes from v1:
	moved from ipt to xt
	get rid of any ipv4 dependencies
	coding style tweaks

 include/linux/netfilter/xt_CHECKSUM.h |   18 ++++++++
 net/netfilter/Kconfig                 |   17 +++++++-
 net/netfilter/Makefile                |    1 +
 net/netfilter/xt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/netfilter/xt_CHECKSUM.h
 create mode 100644 net/netfilter/xt_CHECKSUM.c

diff --git a/include/linux/netfilter/xt_CHECKSUM.h b/include/linux/netfilter/xt_CHECKSUM.h
new file mode 100644
index 0000000..56afe57
--- /dev/null
+++ b/include/linux/netfilter/xt_CHECKSUM.h
@@ -0,0 +1,18 @@
+/* Header file for iptables ipt_CHECKSUM target
+ *
+ * (C) 2002 by Harald Welte <laforge@gnumonks.org>
+ * (C) 2010 Red Hat Inc
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This software is distributed under GNU GPL v2, 1991
+*/
+#ifndef _IPT_CHECKSUM_TARGET_H
+#define _IPT_CHECKSUM_TARGET_H
+
+#define XT_CHECKSUM_OP_FILL	0x01	/* fill in checksum in IP header */
+
+struct xt_CHECKSUM_info {
+	u_int8_t operation;	/* bitset of operations */
+};
+
+#endif /* _IPT_CHECKSUM_TARGET_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 8593a77..1cf4852 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -294,7 +294,7 @@ endif # NF_CONNTRACK
 config NETFILTER_TPROXY
 	tristate "Transparent proxying support (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
-	depends on IP_NF_MANGLE
+	depends on IP_NF_MANGLE || IP6_NF_MANGLE
 	depends on NETFILTER_ADVANCED
 	help
 	  This option enables transparent proxying support, that is,
@@ -347,6 +347,21 @@ config NETFILTER_XT_CONNMARK
 
 comment "Xtables targets"
 
+config NETFILTER_XT_TARGET_CHECKSUM
+	tristate "CHECKSUM target support"
+	depends on NETFILTER_ADVANCED
+	---help---
+	  This option adds a `CHECKSUM' target, which can be used in the iptables mangle
+	  table.  
+
+	  You can use this target to compute and fill in the checksum in
+	  a packet that lacks a checksum.  This is particularly useful,
+	  if you need to work around old applications such as dhcp clients,
+	  that do not work well with checksum offloads, but don't want to disable
+	  checksum offload in your device.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config NETFILTER_XT_TARGET_CLASSIFY
 	tristate '"CLASSIFY" target support'
 	depends on NETFILTER_ADVANCED
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 14e3a8f..8eb541d 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_NETFILTER_XT_MARK) += xt_mark.o
 obj-$(CONFIG_NETFILTER_XT_CONNMARK) += xt_connmark.o
 
 # targets
+obj-$(CONFIG_NETFILTER_XT_TARGET_CHECKSUM) += xt_CHECKSUM.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_CLASSIFY) += xt_CLASSIFY.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_CONNSECMARK) += xt_CONNSECMARK.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_CT) += xt_CT.o
diff --git a/net/netfilter/xt_CHECKSUM.c b/net/netfilter/xt_CHECKSUM.c
new file mode 100644
index 0000000..0fee1a7
--- /dev/null
+++ b/net/netfilter/xt_CHECKSUM.c
@@ -0,0 +1,72 @@
+/* iptables module for the packet checksum mangling
+ *
+ * (C) 2002 by Harald Welte <laforge@netfilter.org>
+ * (C) 2010 Red Hat, Inc.
+ *
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/in.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_CHECKSUM.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Michael S. Tsirkin <mst@redhat.com>");
+MODULE_DESCRIPTION("Xtables: checksum modification");
+MODULE_ALIAS("ipt_CHECKSUM");
+MODULE_ALIAS("ip6t_CHECKSUM");
+
+static unsigned int
+checksum_tg(struct sk_buff *skb, const struct xt_action_param *par)
+{
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		skb_checksum_help(skb);
+
+	return XT_CONTINUE;
+}
+
+static int checksum_tg_check(const struct xt_tgchk_param *par)
+{
+	const struct xt_CHECKSUM_info *einfo = par->targinfo;
+
+	if (einfo->operation & ~XT_CHECKSUM_OP_FILL) {
+		pr_info("unsupported CHECKSUM operation %x\n", einfo->operation);
+		return -EINVAL;
+	}
+	if (!einfo->operation) {
+		pr_info("no CHECKSUM operation enabled\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct xt_target checksum_tg_reg __read_mostly = {
+	.name		= "CHECKSUM",
+	.family		= NFPROTO_UNSPEC,
+	.target		= checksum_tg,
+	.targetsize	= sizeof(struct xt_CHECKSUM_info),
+	.table		= "mangle",
+	.checkentry	= checksum_tg_check,
+	.me		= THIS_MODULE,
+};
+
+static int __init checksum_tg_init(void)
+{
+	return xt_register_target(&checksum_tg_reg);
+}
+
+static void __exit checksum_tg_exit(void)
+{
+	xt_unregister_target(&checksum_tg_reg);
+}
+
+module_init(checksum_tg_init);
+module_exit(checksum_tg_exit);
-- 
1.7.2.rc0.14.g41c1c

[PATCHv3] extensions: libxt_CHECKSUM extension

[Michael S. Tsirkin]

This adds a `CHECKSUM' target, which can be used in the iptables mangle
table.

You can use this target to compute and fill in the checksum in
a packet that lacks a checksum.  This is particularly useful,
if you need to work around old applications such as dhcp clients,
that do not work well with checksum offloads, but don't want to disable
checksum offload in your device.

The problem happens in the field with virtualized applications.
For reference, see Red Hat bz 605555, as well as
http://www.spinics.net/lists/kvm/msg37660.html

Typical expected use (helps old dhclient binary running in a VM):
iptables -A POSTROUTING -t mangle -p udp --dport bootpc \
        -j CHECKSUM --checksum-fill

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Correction in the documentation. Sorry about the noise.

Changes from v2:
	updated man file
Changes from v1:
	switched from ipt to xt

 extensions/libxt_CHECKSUM.c   |   99 +++++++++++++++++++++++++++++++++++++++++
 extensions/libxt_CHECKSUM.man |    8 +++
 2 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 extensions/libxt_CHECKSUM.c
 create mode 100644 extensions/libxt_CHECKSUM.man

diff --git a/extensions/libxt_CHECKSUM.c b/extensions/libxt_CHECKSUM.c
new file mode 100644
index 0000000..00fbd8f
--- /dev/null
+++ b/extensions/libxt_CHECKSUM.c
@@ -0,0 +1,99 @@
+/* Shared library add-on to xtables for CHECKSUM
+ *
+ * (C) 2002 by Harald Welte <laforge@gnumonks.org>
+ * (C) 2010 by Red Hat, Inc
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is distributed under the terms of GNU GPL v2, 1991
+ *
+ * libxt_CHECKSUM.c borrowed some bits from libipt_ECN.c
+ *
+ * $Id$
+ */
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <getopt.h>
+
+#include <xtables.h>
+#include <linux/netfilter/xt_CHECKSUM.h>
+
+static void CHECKSUM_help(void)
+{
+	printf(
+"CHECKSUM target options\n"
+"  --checksum-fill			Fill in packet checksum.\n");
+}
+
+static const struct option CHECKSUM_opts[] = {
+	{ "checksum-fill", 0, NULL, 'F' },
+	{ .name = NULL }
+};
+
+static int CHECKSUM_parse(int c, char **argv, int invert, unsigned int *flags,
+                     const void *entry, struct xt_entry_target **target)
+{
+	struct xt_CHECKSUM_info *einfo
+		= (struct xt_CHECKSUM_info *)(*target)->data;
+
+	switch (c) {
+	case 'F':
+		if (*flags)
+			xtables_error(PARAMETER_PROBLEM,
+			        "CHECKSUM target: Only use --checksum-fill ONCE!");
+		einfo->operation = XT_CHECKSUM_OP_FILL;
+		*flags |= XT_CHECKSUM_OP_FILL;
+		break;
+	default:
+		return 0;
+	}
+
+	return 1;
+}
+
+static void CHECKSUM_check(unsigned int flags)
+{
+	if (!flags)
+		xtables_error(PARAMETER_PROBLEM,
+		           "CHECKSUM target: Parameter --checksum-fill is required");
+}
+
+static void CHECKSUM_print(const void *ip, const struct xt_entry_target *target,
+                      int numeric)
+{
+	const struct xt_CHECKSUM_info *einfo =
+		(const struct xt_CHECKSUM_info *)target->data;
+
+	printf("CHECKSUM ");
+
+	if (einfo->operation & XT_CHECKSUM_OP_FILL)
+		printf("fill ");
+}
+
+static void CHECKSUM_save(const void *ip, const struct xt_entry_target *target)
+{
+	const struct xt_CHECKSUM_info *einfo =
+		(const struct xt_CHECKSUM_info *)target->data;
+
+	if (einfo->operation & XT_CHECKSUM_OP_FILL)
+		printf("--checksum-fill ");
+}
+
+static struct xtables_target checksum_tg_reg = {
+	.name		= "CHECKSUM",
+	.version	= XTABLES_VERSION,
+	.family		= NFPROTO_UNSPEC,
+	.size		= XT_ALIGN(sizeof(struct xt_CHECKSUM_info)),
+	.userspacesize	= XT_ALIGN(sizeof(struct xt_CHECKSUM_info)),
+	.help		= CHECKSUM_help,
+	.parse		= CHECKSUM_parse,
+	.final_check	= CHECKSUM_check,
+	.print		= CHECKSUM_print,
+	.save		= CHECKSUM_save,
+	.extra_opts	= CHECKSUM_opts,
+};
+
+void _init(void)
+{
+	xtables_register_target(&checksum_tg_reg);
+}
diff --git a/extensions/libxt_CHECKSUM.man b/extensions/libxt_CHECKSUM.man
new file mode 100644
index 0000000..92ae700
--- /dev/null
+++ b/extensions/libxt_CHECKSUM.man
@@ -0,0 +1,8 @@
+This target allows to selectively work around broken/old applications.
+It can only be used in the mangle table.
+.TP
+\fB\-\-checksum\-fill\fP
+Compute and fill in the checksum in a packet that lacks a checksum.
+This is particularly useful, if you need to work around old applications
+such as dhcp clients, that do not work well with checksum offloads,
+but don't want to disable checksum offload in your device.
-- 
1.7.2.rc0.14.g41c1c

Re: [PATCHv3] extensions: libxt_CHECKSUM extension

[Patrick McHardy]

Am 11.07.2010 17:14, schrieb Michael S. Tsirkin:
> diff --git a/extensions/libxt_CHECKSUM.c b/extensions/libxt_CHECKSUM.c
> new file mode 100644
> index 0000000..00fbd8f
> --- /dev/null
> +++ b/extensions/libxt_CHECKSUM.c
> @@ -0,0 +1,99 @@
> +/* Shared library add-on to xtables for CHECKSUM
> + *
> + * (C) 2002 by Harald Welte <laforge@gnumonks.org>
> + * (C) 2010 by Red Hat, Inc
> + * Author: Michael S. Tsirkin <mst@redhat.com>
> + *
> + * This program is distributed under the terms of GNU GPL v2, 1991
> + *
> + * libxt_CHECKSUM.c borrowed some bits from libipt_ECN.c
> + *
> + * $Id$

Please no CVS IDs.

> + */
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <getopt.h>
> +
> +#include <xtables.h>
> +#include <linux/netfilter/xt_CHECKSUM.h>
> +
> +static void CHECKSUM_help(void)
> +{
> +	printf(
> +"CHECKSUM target options\n"
> +"  --checksum-fill			Fill in packet checksum.\n");
> +}
> +
> +static const struct option CHECKSUM_opts[] = {
> +	{ "checksum-fill", 0, NULL, 'F' },
> +	{ .name = NULL }
> +};
> +
> +static int CHECKSUM_parse(int c, char **argv, int invert, unsigned int *flags,
> +                     const void *entry, struct xt_entry_target **target)
> +{
> +	struct xt_CHECKSUM_info *einfo
> +		= (struct xt_CHECKSUM_info *)(*target)->data;
> +
> +	switch (c) {
> +	case 'F':
> +		if (*flags)
> +			xtables_error(PARAMETER_PROBLEM,
> +			        "CHECKSUM target: Only use --checksum-fill ONCE!");

There is a helper function called xtables_param_act for checking double
arguments and printing a standarized error message.

> +		einfo->operation = XT_CHECKSUM_OP_FILL;
> +		*flags |= XT_CHECKSUM_OP_FILL;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +

Re: several messages

[Jan Engelhardt]

On Thursday 2010-07-15 11:36, Patrick McHardy wrote:
>> +struct xt_CHECKSUM_info {
>> +	u_int8_t operation;	/* bitset of operations */
>
>Please use __u8 in public header files.
>
>> +#include <linux/in.h>
>
>Do you really need in.h?
>
>> + * $Id$
>
>Please no CVS IDs.
>
>> +	switch (c) {
>> +	case 'F':
>> +		if (*flags)
>> +			xtables_error(PARAMETER_PROBLEM,
>> +			        "CHECKSUM target: Only use --checksum-fill ONCE!");
>
>There is a helper function called xtables_param_act for checking double
>arguments and printing a standarized error message.

I took care of these for Xt-a.

[PATCH 0/1] HID: hid_apple is not used for apple alu wireless keyboards

[Jan Scholz]

Hi Jiri,

While parsing 'hid_blacklist' in hid-core.c my apple alu wireless
keyboard is not found.  This happens because in the blacklist it
is declared with HID_USB_DEVICE although the keyboards are really
bluetooth devices.  The same holds for 'apple_devices' list in
hid-apple.c

This patch fixes it by changing HID_USB_DEVICE to
HID_BLUETOOTH_DEVICE in those two lists.

Jan Scholz (1):
  HID: Apple alu wireless keyboards are bluetooth devices

 drivers/hid/hid-apple.c |    6 +++---
 drivers/hid/hid-core.c  |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

[PATCH 1/1] HID: Apple alu wireless keyboards are bluetooth devices

[Jan Scholz]

Changed HID_USB_DEVICE to HID_BLUETOOTH_DEVICE for the apple alu
wireless keyboards

Signed-off-by: Jan Scholz <Scholz@fias.uni-frankfurt.de>
---
 drivers/hid/hid-apple.c |    6 +++---
 drivers/hid/hid-core.c  |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 9b97795..aa28aed 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -400,12 +400,12 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
 			APPLE_RDESC_JIS },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
 			APPLE_ISO_KEYBOARD },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
 		.driver_data = APPLE_HAS_FN },
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 147ec59..98c7e2d 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1241,9 +1241,9 @@ static const struct hid_device_id hid_blacklist[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS) },
-- 
1.6.0.4

Re: [PATCH 1/1] HID: Apple alu wireless keyboards are bluetooth devices

[Jiri Kosina]

On Wed, 26 Nov 2008, Jan Scholz wrote:

> Changed HID_USB_DEVICE to HID_BLUETOOTH_DEVICE for the apple alu
> wireless keyboards
> Signed-off-by: Jan Scholz <Scholz@fias.uni-frankfurt.de>
> ---
>  drivers/hid/hid-apple.c |    6 +++---
>  drivers/hid/hid-core.c  |    6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 9b97795..aa28aed 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -400,12 +400,12 @@ static const struct hid_device_id apple_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
>  			APPLE_RDESC_JIS },
> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
>  			APPLE_ISO_KEYBOARD },
> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
>  		.driver_data = APPLE_HAS_FN },

Hi Jan,

shouldn't we rather have both USB and Bluetooth variants?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/1] HID: Apple alu wireless keyboards are bluetooth devices

[Jan Scholz]

Jiri Kosina <jkosina@suse.cz> writes:

> On Wed, 26 Nov 2008, Jan Scholz wrote:
>
>> Changed HID_USB_DEVICE to HID_BLUETOOTH_DEVICE for the apple alu
>> wireless keyboards
>> Signed-off-by: Jan Scholz <Scholz@fias.uni-frankfurt.de>
>> ---
>>  drivers/hid/hid-apple.c |    6 +++---
>>  drivers/hid/hid-core.c  |    6 +++---
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
>> index 9b97795..aa28aed 100644
>> --- a/drivers/hid/hid-apple.c
>> +++ b/drivers/hid/hid-apple.c
>> @@ -400,12 +400,12 @@ static const struct hid_device_id apple_devices[] = {
>>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
>>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
>>  			APPLE_RDESC_JIS },
>> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
>> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
>>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
>> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
>>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
>>  			APPLE_ISO_KEYBOARD },
>> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
>> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
>>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
>>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
>>  		.driver_data = APPLE_HAS_FN },
>
> Hi Jan,
>
> shouldn't we rather have both USB and Bluetooth variants?
>
> Thanks,

Hi Jiri,

Hm, I thought the USB_DEVICE_ID_APPLE_ALU_{ANSI,ISO,JIS} were apples usb
aluminum keyboards (standard desktop size), while the
USB_DEVICE_ID_APPLE_ALU_WIRELESS_{ANSI,ISO,JIS} ones were the aluminum
bluetooth keyboards (notebook sized, no numeric keypad, etc).

The one I own is a USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, german layout,
with bluetooth, unfortunately I don't have access to a usb variant.

Jan

Re: [PATCH 1/1] HID: Apple alu wireless keyboards are bluetooth devices

[Jiri Kosina]

On Wed, 26 Nov 2008, Jan Scholz wrote:

> Jiri Kosina <jkosina@suse.cz> writes:
> 
> > On Wed, 26 Nov 2008, Jan Scholz wrote:
> >
> >> Changed HID_USB_DEVICE to HID_BLUETOOTH_DEVICE for the apple alu
> >> wireless keyboards
> >> Signed-off-by: Jan Scholz <Scholz@fias.uni-frankfurt.de>
> >> ---
> >>  drivers/hid/hid-apple.c |    6 +++---
> >>  drivers/hid/hid-core.c  |    6 +++---
> >>  2 files changed, 6 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> >> index 9b97795..aa28aed 100644
> >> --- a/drivers/hid/hid-apple.c
> >> +++ b/drivers/hid/hid-apple.c
> >> @@ -400,12 +400,12 @@ static const struct hid_device_id apple_devices[] = {
> >>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
> >>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> >>  			APPLE_RDESC_JIS },
> >> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
> >> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
> >>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
> >> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
> >> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
> >>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
> >>  			APPLE_ISO_KEYBOARD },
> >> -	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
> >> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
> >>  		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
> >>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
> >>  		.driver_data = APPLE_HAS_FN },
> >
> > Hi Jan,
> >
> > shouldn't we rather have both USB and Bluetooth variants?
> >
> > Thanks,
> 
> Hi Jiri,
> 
> Hm, I thought the USB_DEVICE_ID_APPLE_ALU_{ANSI,ISO,JIS} were apples usb
> aluminum keyboards (standard desktop size), while the
> USB_DEVICE_ID_APPLE_ALU_WIRELESS_{ANSI,ISO,JIS} ones were the aluminum
> bluetooth keyboards (notebook sized, no numeric keypad, etc).
> 
> The one I own is a USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, german layout,
> with bluetooth, unfortunately I don't have access to a usb variant.

Tobias Mueller added these device IDs, so persumably he has tested it and 
could provide some insight. I don't have the hardware myself, so I have no 
idea whether there are only Bluetooht variants, or even USB are available.

Tobias?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/1] HID: Apple alu wireless keyboards are bluetooth devices

[Tobias Müller]

>> The one I own is a USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO, german layout,
>> with bluetooth, unfortunately I don't have access to a usb variant.
>
> Tobias Mueller added these device IDs, so persumably he has tested it and
> could provide some insight. I don't have the hardware myself, so I have no
> idea whether there are only Bluetooht variants, or even USB are available.
>
> Tobias?

I own the USB variant and these are the right id for that. The wireless
IDs were from another patch I merged together with mine. I don't have a
wireless version.

Regards
  Tobias

Re: several messages

[Jiri Kosina]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 406 bytes --]

On Wed, 26 Nov 2008, J.R. Mauro wrote:

> There is one bluetooth model and one USB model.

On Wed, 26 Nov 2008, Tobias Müller wrote:

> I own the USB variant and these are the right id for that. The wireless 
> IDs were from another patch I merged together with mine. I don't have a 
> wireless version.

OK, so therefore you confirm that Jan's patch is OK, right?

Thanks a lot,

-- 
Jiri Kosina
SUSE Labs

[PATCH 1/2] HID: add hid_type

[Jiri Slaby]

Add type to the hid structure to distinguish to which device type
(mouse/kbd) we are talking to. Needed for per device type ignore
list support.

Note: this patch leaves the type as unknown for bluetooth devices,
there is not support for this in the hidp code.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/hid/usbhid/hid-core.c |    8 ++++++++
 include/linux/hid.h           |    7 +++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1d3b8a3..20617d8 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -972,6 +972,14 @@ static int hid_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
 	hid->product = le16_to_cpu(dev->descriptor.idProduct);
 	hid->name[0] = 0;
+	switch (intf->cur_altsetting->desc.bInterfaceProtocol) {
+	case USB_INTERFACE_PROTOCOL_KEYBOARD:
+		hid->type = HID_TYPE_KEYBOARD;
+		break;
+	case USB_INTERFACE_PROTOCOL_MOUSE:
+		hid->type = HID_TYPE_MOUSE;
+		break;
+	}
 
 	if (dev->manufacturer)
 		strlcpy(hid->name, dev->manufacturer, sizeof(hid->name));
diff --git a/include/linux/hid.h b/include/linux/hid.h
index f13bca2..36a3953 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -417,6 +417,12 @@ struct hid_input {
 	struct input_dev *input;
 };
 
+enum hid_type {
+	HID_TYPE_UNKNOWN = 0,
+	HID_TYPE_MOUSE,
+	HID_TYPE_KEYBOARD
+};
+
 struct hid_driver;
 struct hid_ll_driver;
 
@@ -431,6 +437,7 @@ struct hid_device {							/* device report descriptor */
 	__u32 vendor;							/* Vendor ID */
 	__u32 product;							/* Product ID */
 	__u32 version;							/* HID version */
+	enum hid_type type;						/* device type (mouse, kbd, ...) */
 	unsigned country;						/* HID country */
 	struct hid_report_enum report_enum[HID_REPORT_TYPES];
 
-- 
1.6.0.2

[PATCH 2/2] HID: fix appletouch regression

[Jiri Slaby]

The appletouch mouse devices are grabbed by the hid bus and not
released even if apple driver says ENODEV (as expected).

Move the ignoration one level upper to deny the hid layer to grab
the device and return error to the usb hid which, as a result,
releases the device.

Otherwise input/mouse/appletouch and others needn't be attached.

Reported-by: Justin Mattock <justinmattock@gmail.com>
Reported-by: Steven Noonan <steven@uplinklabs.net>
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
---
 drivers/hid/hid-apple.c |   63 ++++++++++++++++------------------------------
 drivers/hid/hid-core.c  |   33 ++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index fd7f896..c6ab4ba 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -312,13 +312,6 @@ static int apple_probe(struct hid_device *hdev,
 	unsigned int connect_mask = HID_CONNECT_DEFAULT;
 	int ret;
 
-	/* return something else or move to hid layer? device will reside
-	   allocated */
-	if (id->bus == BUS_USB && (quirks & APPLE_IGNORE_MOUSE) &&
-			to_usb_interface(hdev->dev.parent)->cur_altsetting->
-			desc.bInterfaceProtocol == USB_INTERFACE_PROTOCOL_MOUSE)
-		return -ENODEV;
-
 	asc = kzalloc(sizeof(*asc), GFP_KERNEL);
 	if (asc == NULL) {
 		dev_err(&hdev->dev, "can't alloc apple descriptor\n");
@@ -367,38 +360,32 @@ static const struct hid_device_id apple_devices[] = {
 		.driver_data = APPLE_MIGHTYMOUSE | APPLE_INVERT_HWHEEL },
 
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_JIS),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ISO),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_ISO_KEYBOARD },
+			APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS},
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ANSI),
 		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_ISO),
@@ -406,14 +393,12 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_JIS),
 		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+			APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO),
@@ -422,25 +407,21 @@ static const struct hid_device_id apple_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_JIS),
 		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO),
-		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_HAS_FN | APPLE_ISO_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS),
-		.driver_data = APPLE_HAS_FN | APPLE_IGNORE_MOUSE | APPLE_RDESC_JIS },
+		.driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY),
-		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN |
-			APPLE_IGNORE_MOUSE },
+		.driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN },
 
 	/* Apple wireless Mighty Mouse */
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, 0x030c),
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8a7d9db..90bdc6f 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1539,6 +1539,35 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ }
 };
 
+/**
+ * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
+ */
+static const struct hid_device_id hid_mouse_ignore_list[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER3_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER4_HF_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_JIS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) },
+	{ }
+};
+
 static bool hid_ignore(struct hid_device *hdev)
 {
 	switch (hdev->vendor) {
@@ -1555,6 +1584,10 @@ static bool hid_ignore(struct hid_device *hdev)
 		break;
 	}
 
+	if (hdev->type == HID_TYPE_MOUSE &&
+			hid_match_id(hdev, hid_mouse_ignore_list))
+		return true;
+
 	return !!hid_match_id(hdev, hid_ignore_list);
 }
 
-- 
1.6.0.2

Re: several messages

[Jiri Kosina]

On Sun, 19 Oct 2008, Jiri Slaby wrote:

> +enum hid_type {
> +	HID_TYPE_UNKNOWN = 0,
> +	HID_TYPE_MOUSE,
> +	HID_TYPE_KEYBOARD
> +};
> +

Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in 
the code. I'd propose to add it when it is actually needed. I.e. have the 
enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add 
whatever will become necessary in the future, what do you think?


On Sun, 19 Oct 2008, Jiri Slaby wrote:

> +/**
> + * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
> + */

I think a more descriptive comment would be appropriate here. It might not 
be obvious on the first sight why this needs to be done separately from 
the generic hid_blacklist. I.e. something like

/** 
 * There are composite devices for which we want to ignore only a certain
 * interface. This is a list of devices for which only the mouse interface 
 * will be ignored.
 */

maybe?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: several messages

[Justin Mattock]

On Sun, Oct 19, 2008 at 12:40 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
>
>> +enum hid_type {
>> +     HID_TYPE_UNKNOWN = 0,
>> +     HID_TYPE_MOUSE,
>> +     HID_TYPE_KEYBOARD
>> +};
>> +
>
> Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in
> the code. I'd propose to add it when it is actually needed. I.e. have the
> enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add
> whatever will become necessary in the future, what do you think?
>
>
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
>
>> +/**
>> + * hid_mouse_ignore_list - mouse devices which must not be held by the hid layer
>> + */
>
> I think a more descriptive comment would be appropriate here. It might not
> be obvious on the first sight why this needs to be done separately from
> the generic hid_blacklist. I.e. something like
>
> /**
>  * There are composite devices for which we want to ignore only a certain
>  * interface. This is a list of devices for which only the mouse interface
>  * will be ignored.
>  */
>
> maybe?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

I can agree with that, whats the point having something
there if it not being used,(just eating up precious space);

-- 
Justin P. Mattock

Re: several messages

[Jiri Slaby]

Jiri Kosina napsal(a):
> On Sun, 19 Oct 2008, Jiri Slaby wrote:
> 
>> +enum hid_type {
>> +	HID_TYPE_UNKNOWN = 0,
>> +	HID_TYPE_MOUSE,
>> +	HID_TYPE_KEYBOARD
>> +};
>> +
> 
> Do we really need the HID_TYPE_KEYBOARD at all? It's not used anywhere in 
> the code. I'd propose to add it when it is actually needed. I.e. have the 
> enum contain something like HID_TYPE_MOUSE HID_TYPE_OTHER for now, and add 
> whatever will become necessary in the future, what do you think?

I would use unknown rather than other, since on bluetooth mouse is unknown
not other, if you don't mind?

Or did you mean tristate unknown, mouse and other?

Thanks for review.

[patch 6/6] rtc suspend()/resume() restores system clock

[David Brownell]

RTC class suspend/resume support, re-initializing the system clock on resume
from the clock used to initialize it at boot time.

 - Inlining the same code used by ARM, which saves and restores the
   delta between a selected RTC and the current system wall-clock time.

 - Removes calls to that ARM code from AT91, OMAP, and S3C RTCs.

This goes on top of the patch series removing "struct class_device" usage
from the RTC framework.  That makes class suspend()/resume() work.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

---
 drivers/rtc/Kconfig          |   24 +++++++++----
 drivers/rtc/class.c          |   74 +++++++++++++++++++++++++++++++++++++++++++
 drivers/rtc/rtc-at91rm9200.c |   30 -----------------
 drivers/rtc/rtc-omap.c       |   17 ---------
 drivers/rtc/rtc-s3c.c        |   22 ------------
 5 files changed, 91 insertions(+), 76 deletions(-)

Index: at91/drivers/rtc/Kconfig
===================================================================
--- at91.orig/drivers/rtc/Kconfig	2007-02-21 18:47:38.000000000 -0800
+++ at91/drivers/rtc/Kconfig	2007-02-21 18:47:41.000000000 -0800
@@ -21,21 +21,31 @@ config RTC_CLASS
 	  will be called rtc-class.
 
 config RTC_HCTOSYS
-	bool "Set system time from RTC on startup"
+	bool "Set system time from RTC on startup and resume"
 	depends on RTC_CLASS = y
 	default y
 	help
-	  If you say yes here, the system time will be set using
-	  the value read from the specified RTC device. This is useful
-	  in order to avoid unnecessary fsck runs.
+	  If you say yes here, the system time (wall clock) will be set using
+	  the value read from a specified RTC device. This is useful to avoid
+	  unnecessary fsck runs at boot time, and to network better.
 
 config RTC_HCTOSYS_DEVICE
-	string "The RTC to read the time from"
+	string "RTC used to set the system time"
 	depends on RTC_HCTOSYS = y
 	default "rtc0"
 	help
-	  The RTC device that will be used as the source for
-	  the system time, usually rtc0.
+	  The RTC device that will be used to (re)initialize the system
+	  clock, usually rtc0.  Initialization is done when the system
+	  starts up, and when it resumes from a low power state.
+	  
+	  This clock should be battery-backed, so that it reads the correct
+	  time when the system boots from a power-off state.  Otherwise, your
+	  system will need an external clock source (like an NTP server).
+
+	  If the clock you specify here is not battery backed, it may still
+	  be useful to reinitialize system time when resuming from system 
+	  sleep states.  Do not specify an RTC here unless it stays powered
+	  during all this system's supported sleep states.
 
 config RTC_DEBUG
 	bool "RTC debug support"
Index: at91/drivers/rtc/class.c
===================================================================
--- at91.orig/drivers/rtc/class.c	2007-02-21 18:47:39.000000000 -0800
+++ at91/drivers/rtc/class.c	2007-02-21 18:47:41.000000000 -0800
@@ -32,6 +32,78 @@ static void rtc_device_release(struct de
 	kfree(rtc);
 }
 
+#if defined(CONFIG_PM) && defined(CONFIG_RTC_HCTOSYS_DEVICE)
+
+/*
+ * On suspend(), measure the delta between one RTC and the
+ * system's wall clock; restore it on resume().
+ */
+
+static struct timespec	delta;
+static time_t		oldtime;
+
+static int rtc_suspend(struct device *dev, pm_message_t mesg)
+{
+	struct rtc_device	*rtc = to_rtc_device(dev);
+	struct rtc_time		tm;
+
+	if (strncmp(rtc->dev.bus_id,
+				CONFIG_RTC_HCTOSYS_DEVICE,
+				BUS_ID_SIZE) != 0)
+		return 0;
+
+	rtc_read_time(rtc, &tm);
+	rtc_tm_to_time(&tm, &oldtime);
+
+	/* RTC precision is 1 second; adjust delta for avg 1/2 sec err */
+	set_normalized_timespec(&delta,
+				xtime.tv_sec - oldtime,
+				xtime.tv_nsec - (NSEC_PER_SEC >> 1));
+
+	return 0;
+}
+
+static int rtc_resume(struct device *dev)
+{
+	struct rtc_device	*rtc = to_rtc_device(dev);
+	struct rtc_time		tm;
+	time_t			newtime;
+	struct timespec		time;
+
+	if (strncmp(rtc->dev.bus_id,
+				CONFIG_RTC_HCTOSYS_DEVICE,
+				BUS_ID_SIZE) != 0)
+		return 0;
+
+	rtc_read_time(rtc, &tm);
+	if (rtc_valid_tm(&tm) != 0) {
+		pr_debug("%s:  bogus resume time\n", rtc->dev.bus_id);
+		return 0;
+	}
+	rtc_tm_to_time(&tm, &newtime);
+	if (newtime <= oldtime) {
+		if (newtime < oldtime)
+			pr_debug("%s:  time travel!\n", rtc->dev.bus_id);
+		return 0;
+	}
+
+	/* restore wall clock using delta against this RTC;
+	 * adjust again for avg 1/2 second RTC sampling error
+	 */
+	set_normalized_timespec(&time,
+				newtime + delta.tv_sec,
+				(NSEC_PER_SEC >> 1) + delta.tv_nsec);
+	do_settimeofday(&time);
+
+	return 0;
+}
+
+#else
+#define rtc_suspend	NULL
+#define rtc_resume	NULL
+#endif
+
+
 /**
  * rtc_device_register - register w/ RTC class
  * @dev: the device to register
@@ -138,6 +210,8 @@ static int __init rtc_init(void)
 		printk(KERN_ERR "%s: couldn't create class\n", __FILE__);
 		return PTR_ERR(rtc_class);
 	}
+	rtc_class->suspend = rtc_suspend;
+	rtc_class->resume = rtc_resume;
 	rtc_dev_init();
 	rtc_sysfs_init(rtc_class);
 	return 0;
Index: at91/drivers/rtc/rtc-at91rm9200.c
===================================================================
--- at91.orig/drivers/rtc/rtc-at91rm9200.c	2007-02-21 18:47:26.000000000 -0800
+++ at91/drivers/rtc/rtc-at91rm9200.c	2007-02-21 18:47:41.000000000 -0800
@@ -348,21 +348,10 @@ static int __exit at91_rtc_remove(struct
 
 /* AT91RM9200 RTC Power management control */
 
-static struct timespec at91_rtc_delta;
 static u32 at91_rtc_imr;
 
 static int at91_rtc_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
-	/* calculate time delta for suspend */
-	at91_rtc_readtime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	save_time_delta(&at91_rtc_delta, &time);
-
 	/* this IRQ is shared with DBGU and other hardware which isn't
 	 * necessarily doing PM like we are...
 	 */
@@ -374,36 +363,17 @@ static int at91_rtc_suspend(struct platf
 		else
 			at91_sys_write(AT91_RTC_IDR, at91_rtc_imr);
 	}
-
-	pr_debug("%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __FUNCTION__,
-		1900 + tm.tm_year, tm.tm_mon, tm.tm_mday,
-		tm.tm_hour, tm.tm_min, tm.tm_sec);
-
 	return 0;
 }
 
 static int at91_rtc_resume(struct platform_device *pdev)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
-	at91_rtc_readtime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	restore_time_delta(&at91_rtc_delta, &time);
-
 	if (at91_rtc_imr) {
 		if (device_may_wakeup(&pdev->dev))
 			disable_irq_wake(AT91_ID_SYS);
 		else
 			at91_sys_write(AT91_RTC_IER, at91_rtc_imr);
 	}
-
-	pr_debug("%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __FUNCTION__,
-		1900 + tm.tm_year, tm.tm_mon, tm.tm_mday,
-		tm.tm_hour, tm.tm_min, tm.tm_sec);
-
 	return 0;
 }
 #else
Index: at91/drivers/rtc/rtc-omap.c
===================================================================
--- at91.orig/drivers/rtc/rtc-omap.c	2007-02-21 18:47:39.000000000 -0800
+++ at91/drivers/rtc/rtc-omap.c	2007-02-21 18:47:41.000000000 -0800
@@ -488,19 +488,10 @@ static int __devexit omap_rtc_remove(str
 
 #ifdef CONFIG_PM
 
-static struct timespec rtc_delta;
 static u8 irqstat;
 
 static int omap_rtc_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct rtc_time rtc_tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-	omap_rtc_read_time(NULL, &rtc_tm);
-	rtc_tm_to_time(&rtc_tm, &time.tv_sec);
-
-	save_time_delta(&rtc_delta, &time);
 	irqstat = rtc_read(OMAP_RTC_INTERRUPTS_REG);
 
 	/* FIXME the RTC alarm is not currently acting as a wakeup event
@@ -517,14 +508,6 @@ static int omap_rtc_suspend(struct platf
 
 static int omap_rtc_resume(struct platform_device *pdev)
 {
-	struct rtc_time rtc_tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-	omap_rtc_read_time(NULL, &rtc_tm);
-	rtc_tm_to_time(&rtc_tm, &time.tv_sec);
-
-	restore_time_delta(&rtc_delta, &time);
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(omap_rtc_alarm);
 	else
Index: at91/drivers/rtc/rtc-s3c.c
===================================================================
--- at91.orig/drivers/rtc/rtc-s3c.c	2007-02-21 18:47:26.000000000 -0800
+++ at91/drivers/rtc/rtc-s3c.c	2007-02-21 18:47:41.000000000 -0800
@@ -548,37 +548,15 @@ static int ticnt_save;
 
 static int s3c_rtc_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
 	/* save TICNT for anyone using periodic interrupts */
-
 	ticnt_save = readb(s3c_rtc_base + S3C2410_TICNT);
-
-	/* calculate time delta for suspend */
-
-	s3c_rtc_gettime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	save_time_delta(&s3c_rtc_delta, &time);
 	s3c_rtc_enable(pdev, 0);
-
 	return 0;
 }
 
 static int s3c_rtc_resume(struct platform_device *pdev)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
 	s3c_rtc_enable(pdev, 1);
-	s3c_rtc_gettime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	restore_time_delta(&s3c_rtc_delta, &time);
-
 	writeb(ticnt_save, s3c_rtc_base + S3C2410_TICNT);
 	return 0;
 }

Re: several messages

[Guennadi Liakhovetski]

of the following 2 patches:

On Mon, 5 Feb 2007, Johannes Berg wrote:

> This patch removes the time suspend/restore code that was done through
> a PMU notifier in arch/platforms/powermac/time.c.
> 
> Instead, we introduce arch/powerpc/sysdev/timer.c which creates a sys
> device and handles time of day suspend/resume through that.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> Cc: Andrew Morton <akpm@osdl.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

[patch trimmed]

On Wed, 21 Feb 2007, David Brownell wrote:

> RTC class suspend/resume support, re-initializing the system clock on resume
> >from the clock used to initialize it at boot time.
> 
>  - Inlining the same code used by ARM, which saves and restores the
>    delta between a selected RTC and the current system wall-clock time.
> 
>  - Removes calls to that ARM code from AT91, OMAP, and S3C RTCs.
> 
> This goes on top of the patch series removing "struct class_device" usage
> >from the RTC framework.  That makes class suspend()/resume() work.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> 
> ---
>  drivers/rtc/Kconfig          |   24 +++++++++----
>  drivers/rtc/class.c          |   74 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/rtc/rtc-at91rm9200.c |   30 -----------------
>  drivers/rtc/rtc-omap.c       |   17 ---------
>  drivers/rtc/rtc-s3c.c        |   22 ------------
>  5 files changed, 91 insertions(+), 76 deletions(-)

[patch trimmed]

I think, we only want 1, right? And the latter seems to be more generic / 
platform independent? And as a side-effect, powermac would have to migrate 
to generic rtc:-)

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: several messages

[David Brownell]

On Thursday 22 February 2007 2:58 pm, Guennadi Liakhovetski wrote:
> 
> I think, we only want 1, right? And the latter seems to be more generic / 
> platform independent? And as a side-effect, powermac would have to migrate 
> to generic rtc:-)

I'd certainly think that restoring the system clock should be, as much
as possible, in platform-agnostic code.  Like the generic RTC framework.

And hmm, that powermac/time.c file replicates other RTC code...

Minor obstacle:  removing the EXPERIMENTAL label from that code.

- Dave

Re: several messages

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

On Thu, 2007-02-22 at 23:58 +0100, Guennadi Liakhovetski wrote:

> I think, we only want 1, right? And the latter seems to be more generic / 
> platform independent? And as a side-effect, powermac would have to migrate 
> to generic rtc:-)

Can we migrate all of powerpc to genrtc? But yes, I agree. Had enough to
do though already to get suspend working :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Linux 2.6.16.4

[Greg KH]

We (the -stable team) are announcing the release of the 2.6.16.4 kernel.

The diffstat and short summary of the fixes are below.

I'll also be replying to this message with a copy of the patch between
2.6.16.3 and 2.6.16.4, as it is small enough to do so.

The updated 2.6.16.y git tree can be found at:
 	rsync://rsync.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.16.y.git
and can be browsed at the normal kernel.org git web browser:
	www.kernel.org/git/

thanks,

greg k-h

--------

 Makefile        |    2 +-
 kernel/signal.c |    1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

Summary of changes from v2.6.16.3 to v2.6.16.4
==============================================


Greg Kroah-Hartman:
      Linux 2.6.16.4

Oleg Nesterov:
      RCU signal handling [CVE-2006-1523]

Re: several messages

[Jan Engelhardt]

>Date: Tue, 11 Apr 2006 09:26:20 -0700
>Subject: Linux 2.6.16.3
>David Howells:
>      Keys: Fix oops when adding key to non-keyring [CVE-2006-1522]

>Date: Tue, 11 Apr 2006 10:33:23 -0700
>Subject: Linux 2.6.16.4
>Oleg Nesterov:
>      RCU signal handling [CVE-2006-1523]

Now admins spend another hour this day just to upgrade.
These two patches could have been queued until the end of the day. Maybe 
another one turns up soon.


Jan Engelhardt
-- 

Re: several messages

[Boris B. Zhmurov]

Hello, Jan Engelhardt.

On 11.04.2006 23:04 you said the following:


> Now admins spend another hour this day just to upgrade.

It's admin's job, isn't it?


> These two patches could have been queued until the end of the day. Maybe 
> another one turns up soon.
> Jan Engelhardt

Hmm... Interesting. Are you blaming security officers for doing their 
job? Please, don't! And many many thanks to Greg for giving us security 
patches as soon as possible.


-- 
Boris B. Zhmurov
mailto: bb@kernelpanic.ru
"wget http://kernelpanic.ru/bb_public_key.pgp -O - | gpg --import"

Re: several messages

[Greg KH]

On Tue, Apr 11, 2006 at 09:04:42PM +0200, Jan Engelhardt wrote:
> 
> >Date: Tue, 11 Apr 2006 09:26:20 -0700
> >Subject: Linux 2.6.16.3
> >David Howells:
> >      Keys: Fix oops when adding key to non-keyring [CVE-2006-1522]
> 
> >Date: Tue, 11 Apr 2006 10:33:23 -0700
> >Subject: Linux 2.6.16.4
> >Oleg Nesterov:
> >      RCU signal handling [CVE-2006-1523]
> 
> Now admins spend another hour this day just to upgrade.
> These two patches could have been queued until the end of the day. Maybe 
> another one turns up soon.

The first one went out last night, as it was a real issue that affected
people and I had already waited longer than I felt comfortable with, due
to travel issues I had (two different talks in two different cities in
two different days.)

The second one went out today, because it was reported today.  Should I
have waited until tomorrow to see if something else came up?

thanks,

greg k-h

Re: several messages

[Jan Engelhardt]

>
>The first one went out last night, as it was a real issue that affected
>people and I had already waited longer than I felt comfortable with, due
>to travel issues I had (two different talks in two different cities in
>two different days.)
>
>The second one went out today, because it was reported today.  Should I
>have waited until tomorrow to see if something else came up?
>
No of course not, I did not know the first one was already due long time.


[Sigh, pine changed the subject header and I did not notice.]


Jan Engelhardt
-- 

Re: several messages

[Nix]

On 11 Apr 2006, Greg KH whispered secretively:
> The first one went out last night, as it was a real issue that affected
> people and I had already waited longer than I felt comfortable with, due
> to travel issues I had (two different talks in two different cities in
> two different days.)
> 
> The second one went out today, because it was reported today.  Should I
> have waited until tomorrow to see if something else came up?

Indeed.

On top of that, they're `only' local DoSes, so many admins (i.e. those
without untrusted local users) will probably not have installed .3 yet:
and anyone with untrusted local users probably has someone whose entire
job is handling security anyway.


There's nothing wrong with rapid-fire -stables; either the issue is (in
the judgement of the ones doing the installation) critical, in which
case it should get out as fast as possible, or it isn't, in which case
the local installing admins can put it off for a day or so themselves to
see if another release comes out immediately afterwards.

-- 
`On a scale of 1-10, X's "brokenness rating" is 1.1, but that's only
 because bringing Windows into the picture rescaled "brokenness" by
 a factor of 10.' --- Peter da Silva

ata over ethernet question

[Maciej Soltysiak]

Hi,

AOE is a bit new for me.

Would it be possible to use tha AOE driver to
attach one ATA drive in a host over ethernet to another
host ? Or is it support for specific hardware devices only?

You know, something like:
# fdisk <device_on_another_host>
# mkfs.ext2 <device_on_another_host/partition1>
# mount <device_on_another_host/partition1> /mnt/part1

--
Maciej

Re: ata over ethernet question

[David Hollis]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

On Wed, 2005-05-04 at 19:31 +0200, Maciej Soltysiak wrote:
> Hi,
> 
> AOE is a bit new for me.
> 
> Would it be possible to use tha AOE driver to
> attach one ATA drive in a host over ethernet to another
> host ? Or is it support for specific hardware devices only?
> 
> You know, something like:
> # fdisk <device_on_another_host>
> # mkfs.ext2 <device_on_another_host/partition1>
> # mount <device_on_another_host/partition1> /mnt/part1
> 

That seems to be the basic idea but there doesn't seem to be a provider
stack just yet, just a 'client' (though I could be wrong).  AOE is
similar in concept to iSCSI with the biggest difference being that AOE
runs over Ethernet and is thus non-routeable.  iSCSI operates over IP so
you can do all kinds of fun IP games with it.

> --
> Maciej
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
David Hollis <dhollis@davehollis.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re[2]: ata over ethernet question

[Maciej Soltysiak]

Hello David,

Wednesday, May 4, 2005, 9:48:36 PM, you wrote:
> That seems to be the basic idea but there doesn't seem to be a provider
> stack just yet, just a 'client' (though I could be wrong).  AOE is
> similar in concept to iSCSI with the biggest difference being that AOE
> runs over Ethernet and is thus non-routeable.  iSCSI operates over IP so
> you can do all kinds of fun IP games with it.
Thanks, this is interesting. Does the iSCSI implementation out there have
this provider stack ?

Regards,
Maciej

Re: Re[2]: ata over ethernet question

[David Hollis]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

On Wed, 2005-05-04 at 23:17 +0200, Maciej Soltysiak wrote:
> Hello David,
> 
> Wednesday, May 4, 2005, 9:48:36 PM, you wrote:
> > That seems to be the basic idea but there doesn't seem to be a provider
> > stack just yet, just a 'client' (though I could be wrong).  AOE is
> > similar in concept to iSCSI with the biggest difference being that AOE
> > runs over Ethernet and is thus non-routeable.  iSCSI operates over IP so
> > you can do all kinds of fun IP games with it.
> Thanks, this is interesting. Does the iSCSI implementation out there have
> this provider stack ?
> 
> Regards,
> Maciej

There seem to be a few iSCSI implementations floating around for Linux,
hopefully one will be added to mainline soon.  Most of those
implementations are for the client side though there is at least one
target implementation that allows you to provide local storage to iSCSI
clients.  I don't remember the name of it or if it's still maintained or
not.

-- 
David Hollis <dhollis@davehollis.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Re[2]: ata over ethernet question

[Sander]

David Hollis wrote (ao):
> There seem to be a few iSCSI implementations floating around for
> Linux, hopefully one will be added to mainline soon. Most of those
> implementations are for the client side though there is at least one
> target implementation that allows you to provide local storage to
> iSCSI clients. I don't remember the name of it or if it's still
> maintained or not.

Quite active even:

http://sourceforge.net/projects/iscsitarget/

The "Quick Guide to iSCSI on Linux" is a good starting point btw.

Also check out http://www.open-iscsi.org/ (the client, aka 'initiator').

-- 
Humilis IT Services and Solutions
http://www.humilis.net

Re: Re[2]: ata over ethernet question

[Guennadi Liakhovetski]

Hi

On Sat, 7 May 2005, Sander wrote:

> David Hollis wrote (ao):
> > There seem to be a few iSCSI implementations floating around for
> > Linux, hopefully one will be added to mainline soon. Most of those
> > implementations are for the client side though there is at least one
> > target implementation that allows you to provide local storage to
> > iSCSI clients. I don't remember the name of it or if it's still
> > maintained or not.
> 
> Quite active even:
> 
> http://sourceforge.net/projects/iscsitarget/
> 
> The "Quick Guide to iSCSI on Linux" is a good starting point btw.
> 
> Also check out http://www.open-iscsi.org/ (the client, aka 'initiator').

A follow up question - I recently used nbd to access a CD-ROM. It worked 
nice, but, I had to read in 7 CDs, so, each time I had to replace a CD, I 
had to stop the client, the server, then replace the CD, re-start the 
server, re-start the client... I thought about extending NBD to (better) 
support removable media, but then you start thinking about all those 
features that your local block device has that don't get exported over 
NBD...

Now, my understanding (sorry, without looking at any docs - yet) is, that 
iSCSI is (or at least should be) free from these limitations. So, does it 
make any sense at all extending NBD or just switch to iSCSI? Should NBD be 
just kept simple as it is or would it be completely superseeded by iSCSI, 
or is there still something that NBD does that iSCSI wouldn't (easily) do?

Or am I completely misunderstanding what iSCSI target does?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: ata over ethernet question

[Vladislav Bolkhovitin]

Guennadi Liakhovetski wrote:
> Hi
> 
> On Sat, 7 May 2005, Sander wrote:
> 
> 
>>David Hollis wrote (ao):
>>
>>>There seem to be a few iSCSI implementations floating around for
>>>Linux, hopefully one will be added to mainline soon. Most of those
>>>implementations are for the client side though there is at least one
>>>target implementation that allows you to provide local storage to
>>>iSCSI clients. I don't remember the name of it or if it's still
>>>maintained or not.
>>
>>Quite active even:
>>
>>http://sourceforge.net/projects/iscsitarget/
>>
>>The "Quick Guide to iSCSI on Linux" is a good starting point btw.
>>
>>Also check out http://www.open-iscsi.org/ (the client, aka 'initiator').
> 
> 
> A follow up question - I recently used nbd to access a CD-ROM. It worked 
> nice, but, I had to read in 7 CDs, so, each time I had to replace a CD, I 
> had to stop the client, the server, then replace the CD, re-start the 
> server, re-start the client... I thought about extending NBD to (better) 
> support removable media, but then you start thinking about all those 
> features that your local block device has that don't get exported over 
> NBD...
> 
> Now, my understanding (sorry, without looking at any docs - yet) is, that 
> iSCSI is (or at least should be) free from these limitations. So, does it 
> make any sense at all extending NBD or just switch to iSCSI? Should NBD be 
> just kept simple as it is or would it be completely superseeded by iSCSI, 
> or is there still something that NBD does that iSCSI wouldn't (easily) do?
> 
> Or am I completely misunderstanding what iSCSI target does?

Actually, this is property not of iSCSI target itself, but of any SCSI 
target. So, we implemented it as part of our SCSI target mid-level 
(SCST, http://scst.sourceforge.net), therefore any target driver working 
over it will automatically benefit from this feature. Unfortunately, 
currently available only target drivers for Qlogic 2x00 cards and for 
poor UNH iSCSI target (that works not too reliable and only with very 
specific initiators). The published version supports only real SCSI 
CDROMs. CDROM FILEIO module, which allows exporting ISO images as SCSI 
CDROM devices, going to be available not later end of May.

Vlad

> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

Re: several messages

[Guennadi Liakhovetski]

Hello and thanks for the replies

On Wed, 11 May 2005, FUJITA Tomonori wrote:
> The iSCSI protocol simply encapsulates the SCSI protocol into the
> TCP/IP protocol, and carries packets over IP networks. You can handle
...

On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> Actually, this is property not of iSCSI target itself, but of any SCSI target.
> So, we implemented it as part of our SCSI target mid-level (SCST,
> http://scst.sourceforge.net), therefore any target driver working over it will
> automatically benefit from this feature. Unfortunately, currently available
> only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
> works not too reliable and only with very specific initiators). The published
...

The above confirms basically my understanding apart from one "minor" 
confusion - I thought, that parallel to hardware solutions pure software 
implementations were possible / being developed, like a driver, that 
implements a SCSI LDD API on one side, and forwards packets to an IP 
stack, say, over an ethernet card - on the initiator side. And a counter 
part on the target side. Similarly to the USB mass-storage and storage 
gadget drivers?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: several messages

[Ming Zhang]

[-- Attachment #1: Type: text/plain, Size: 1703 bytes --]

iscsi is scsi over ip.
usb disk is scsi over usb.
so just a different transport.
u are rite. ;)

ming

On Wed, 2005-05-11 at 23:26 +0200, Guennadi Liakhovetski wrote:
> Hello and thanks for the replies
> 
> On Wed, 11 May 2005, FUJITA Tomonori wrote:
> > The iSCSI protocol simply encapsulates the SCSI protocol into the
> > TCP/IP protocol, and carries packets over IP networks. You can handle
> ...
> 
> On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> > Actually, this is property not of iSCSI target itself, but of any SCSI target.
> > So, we implemented it as part of our SCSI target mid-level (SCST,
> > http://scst.sourceforge.net), therefore any target driver working over it will
> > automatically benefit from this feature. Unfortunately, currently available
> > only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
> > works not too reliable and only with very specific initiators). The published
> ...
> 
> The above confirms basically my understanding apart from one "minor" 
> confusion - I thought, that parallel to hardware solutions pure software 
> implementations were possible / being developed, like a driver, that 
> implements a SCSI LDD API on one side, and forwards packets to an IP 
> stack, say, over an ethernet card - on the initiator side. And a counter 
> part on the target side. Similarly to the USB mass-storage and storage 
> gadget drivers?
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: several messages

[Dmitry Yusupov]

On Wed, 2005-05-11 at 22:16 -0400, Ming Zhang wrote:
> iscsi is scsi over ip.

correction. iSCSI today has RFC at least for two transports - TCP/IP and
iSER/RDMA(in finalized progress) with RDMA over Infiniband or RNIC. And
I think people start writing initial draft for SCTP/IP transport...

>From this perspective, iSCSI certainly more advanced and matured
comparing to NBD variations. 

> usb disk is scsi over usb.
> so just a different transport.
> u are rite. ;)
> 
> ming
> 
> On Wed, 2005-05-11 at 23:26 +0200, Guennadi Liakhovetski wrote:
> > Hello and thanks for the replies
> > 
> > On Wed, 11 May 2005, FUJITA Tomonori wrote:
> > > The iSCSI protocol simply encapsulates the SCSI protocol into the
> > > TCP/IP protocol, and carries packets over IP networks. You can handle
> > ...
> > 
> > On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> > > Actually, this is property not of iSCSI target itself, but of any SCSI target.
> > > So, we implemented it as part of our SCSI target mid-level (SCST,
> > > http://scst.sourceforge.net), therefore any target driver working over it will
> > > automatically benefit from this feature. Unfortunately, currently available
> > > only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
> > > works not too reliable and only with very specific initiators). The published
> > ...
> > 
> > The above confirms basically my understanding apart from one "minor" 
> > confusion - I thought, that parallel to hardware solutions pure software 
> > implementations were possible / being developed, like a driver, that 
> > implements a SCSI LDD API on one side, and forwards packets to an IP 
> > stack, say, over an ethernet card - on the initiator side. And a counter 
> > part on the target side. Similarly to the USB mass-storage and storage 
> > gadget drivers?
> > 
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski
> > 
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: several messages

[Christoph Hellwig]

On Thu, May 12, 2005 at 11:32:12AM -0700, Dmitry Yusupov wrote:
> On Wed, 2005-05-11 at 22:16 -0400, Ming Zhang wrote:
> > iscsi is scsi over ip.
> 
> correction. iSCSI today has RFC at least for two transports - TCP/IP and
> iSER/RDMA(in finalized progress) with RDMA over Infiniband or RNIC. And
> I think people start writing initial draft for SCTP/IP transport...
> 
> >From this perspective, iSCSI certainly more advanced and matured
> comparing to NBD variations. 

It's for certainly much more complicated (in marketing speak that's usually
called advanced) but far less mature.

If you want network storage to just work use nbd.

Re: several messages

[Dmitry Yusupov]

On Fri, 2005-05-13 at 09:12 +0100, Christoph Hellwig wrote:
> On Thu, May 12, 2005 at 11:32:12AM -0700, Dmitry Yusupov wrote:
> > On Wed, 2005-05-11 at 22:16 -0400, Ming Zhang wrote:
> > > iscsi is scsi over ip.
> > 
> > correction. iSCSI today has RFC at least for two transports - TCP/IP and
> > iSER/RDMA(in finalized progress) with RDMA over Infiniband or RNIC. And
> > I think people start writing initial draft for SCTP/IP transport...
> > 
> > >From this perspective, iSCSI certainly more advanced and matured
> > comparing to NBD variations. 
> 
> It's for certainly much more complicated (in marketing speak that's usually
> called advanced) but far less mature.
> 
> If you want network storage to just work use nbd.

You could tell this to school's computer class teacher... Serious SAN
deployment will always be based either on FC or iSCSI for the reasons I
explained before.

I do not disagree, nbd is nice and simple and for sure has its own
deployment space.

Dmitry

Re: several messages

[Christoph Hellwig]

On Fri, May 13, 2005 at 08:04:16AM -0700, Dmitry Yusupov wrote:
> You could tell this to school's computer class teacher... Serious SAN
> deployment will always be based either on FC or iSCSI for the reasons I
> explained before.

Just FYI Steeleye ships a very successful clustering product that builds
on nbd.

Re: several messages

[Dmitry Yusupov]

On Fri, 2005-05-13 at 16:07 +0100, Christoph Hellwig wrote:
> On Fri, May 13, 2005 at 08:04:16AM -0700, Dmitry Yusupov wrote:
> > You could tell this to school's computer class teacher... Serious SAN
> > deployment will always be based either on FC or iSCSI for the reasons I
> > explained before.
> 
> Just FYI Steeleye ships a very successful clustering product that builds
> on nbd.

AFAIK, it is used for Data Replication purposes only. Correct me if I'm
wrong...

Re: several messages

[Vladislav Bolkhovitin]

Guennadi Liakhovetski wrote:
> Hello and thanks for the replies
> 
> On Wed, 11 May 2005, FUJITA Tomonori wrote:
> 
>>The iSCSI protocol simply encapsulates the SCSI protocol into the
>>TCP/IP protocol, and carries packets over IP networks. You can handle
> 
> ...
> 
> On Wed, 11 May 2005, Vladislav Bolkhovitin wrote:
> 
>>Actually, this is property not of iSCSI target itself, but of any SCSI target.
>>So, we implemented it as part of our SCSI target mid-level (SCST,
>>http://scst.sourceforge.net), therefore any target driver working over it will
>>automatically benefit from this feature. Unfortunately, currently available
>>only target drivers for Qlogic 2x00 cards and for poor UNH iSCSI target (that
>>works not too reliable and only with very specific initiators). The published
> 
> ...
> 
> The above confirms basically my understanding apart from one "minor" 
> confusion - I thought, that parallel to hardware solutions pure software 
> implementations were possible / being developed, like a driver, that 
> implements a SCSI LDD API on one side, and forwards packets to an IP 
> stack, say, over an ethernet card - on the initiator side. And a counter 
> part on the target side. Similarly to the USB mass-storage and storage 
> gadget drivers?

There is some confusion in the SCSI world between SCSI as a transport 
and SCSI as a commands set and software communication protocol, which 
works above the transport. So, you can implement SCSI transport at any 
software (eg iSCSI) or hardware (parallel SCSI, Fibre Channel, SATA, 
etc.) way, but if the SCSI message passing protocol is used overall 
system remains SCSI with all protocol obligations like task management. 
So, pure software SCSI solution is possible. BTW, there are pure 
hardware iSCSI implementations as well.

Vlad

> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

Re: 2.6.6-rc3-mm2 (4KSTACK)

[Helge Hafting]

Bill Davidsen wrote:

> Arjan van de Ven wrote:
>
>>> It's probably a Bad Idea to push this to Linus before the vendors 
>>> that have
>>> significant market-impact issues (again - anybody other than NVidia 
>>> here?)
>>> have gotten their stuff cleaned up...
>>
>>
>>
>> Ok I don't want to start a flamewar but... Do we want to hold linux back
>> until all binary only module vendors have caught up ??
>
>
> My questions is, hold it back from what? Having the 4k option is fine, 
> it's just eliminating the current default which I think is 
> undesirable. I tried 4k stack, I couldn't measure any improvement in 
> anything (as in no visible speedup or saving in memory). 

The memory saving is usually modest: 4k per thread. It might make a 
difference for
those with many thousands of threads.   I believe this is unswappable 
memory,
which is much more valuable than ordinary process memory.

More interesting is that it removes one way for fork() to fail. With 8k 
stacks,
the new process needs to allocate two consecutive pages for those 8k.  That
might be impossible due to fragmentation, even if there are megabytes of 
free
memory. Such a problem usually only shows up after a long time.  Now we 
only need to
allocate a single page, which always works as long as there is any free 
memory at all.

> For an embedded system, where space is tight and the code paths well 
> known, sure, but I haven't been able to find or generate any objective 
> improvement, other than some posts saying smaller is always better. 
> Nothing slows a system down like a crash, even if it isn't followed by 
> a restore from backup.

Consider the case when your server (web/mail/other) fails to fork, and then
you can't login because that requires fork() too.  4k stacks remove this 
scenario,
and is a stability improvement.

Helge Hafting

Re: several messages

[Bill Davidsen]

Thank all of you for this information. This is an interesting way to
overcome the kernel memory fragmentation issue. I would have thought it
was more important to address having the memory so fragmented that there
is no 8k chunk left "even with many megabytes free" as someone wrote.


On Mon, 10 May 2004, Horst von Brand wrote:

> Bill Davidsen <davidsen@tmr.com> said:
> 
> [...]
> 
> > I tried 4k stack, I couldn't measure any improvement in anything (as in 
> > no visible speedup or saving in memory).
> 
> 4K stacks lets the kernel create new threads/processes as long as there is
> free memory; with 8K stacks it needs two consecutive free page frames in
> physical memory, when memory is fragmented (and large) they are hard to
> come by...
> -- 
> Dr. Horst H. von Brand                   User #22616 counter.li.org
> Departamento de Informatica                     Fono: +56 32 654431
> Universidad Tecnica Federico Santa Maria              +56 32 654239
> Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513
> 

On Mon, 10 May 2004, Andrew Morton wrote:

> Horst von Brand <vonbrand@inf.utfsm.cl> wrote:
> >
> > Bill Davidsen <davidsen@tmr.com> said:
> > 
> > [...]
> > 
> > > I tried 4k stack, I couldn't measure any improvement in anything (as in 
> > > no visible speedup or saving in memory).
> > 
> > 4K stacks lets the kernel create new threads/processes as long as there is
> > free memory; with 8K stacks it needs two consecutive free page frames in
> > physical memory, when memory is fragmented (and large) they are hard to
> > come by...
> 
> This is true to a surprising extent.  A couple of weeks ago I observed my
> 256MB box freeing over 20MB of pages before it could successfully acquire a
> single 1-order page.
> 
> That was during an updatedb run.
> 
> And a 1-order GFP_NOFS allocation was actually livelocking, because
> !__GFP_FS allocations aren't allowed to enter dentry reclaim.  Which is why
> VFS caches are now forced to use 0-order allocations.
> 
> 

On Tue, 11 May 2004, Helge Hafting wrote:

> Bill Davidsen wrote:
> 
> > Arjan van de Ven wrote:
> >
> >>> It's probably a Bad Idea to push this to Linus before the vendors 
> >>> that have
> >>> significant market-impact issues (again - anybody other than NVidia 
> >>> here?)
> >>> have gotten their stuff cleaned up...
> >>
> >>
> >>
> >> Ok I don't want to start a flamewar but... Do we want to hold linux back
> >> until all binary only module vendors have caught up ??
> >
> >
> > My questions is, hold it back from what? Having the 4k option is fine, 
> > it's just eliminating the current default which I think is 
> > undesirable. I tried 4k stack, I couldn't measure any improvement in 
> > anything (as in no visible speedup or saving in memory). 
> 
> The memory saving is usually modest: 4k per thread. It might make a 
> difference for
> those with many thousands of threads.   I believe this is unswappable 
> memory,
> which is much more valuable than ordinary process memory.
> 
> More interesting is that it removes one way for fork() to fail. With 8k 
> stacks,
> the new process needs to allocate two consecutive pages for those 8k.  That
> might be impossible due to fragmentation, even if there are megabytes of 
> free
> memory. Such a problem usually only shows up after a long time.  Now we 
> only need to
> allocate a single page, which always works as long as there is any free 
> memory at all.
> 
> > For an embedded system, where space is tight and the code paths well 
> > known, sure, but I haven't been able to find or generate any objective 
> > improvement, other than some posts saying smaller is always better. 
> > Nothing slows a system down like a crash, even if it isn't followed by 
> > a restore from backup.
> 
> Consider the case when your server (web/mail/other) fails to fork, and then
> you can't login because that requires fork() too.  4k stacks remove this 
> scenario,
> and is a stability improvement.
> 
> Helge Hafting
> 

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: ANN: LKMB (Linux Kernel Module Builder) version 0.1.16

[Hal Duston]

I use "INSTALL_MOD_PATH=put/the/modules/here/instead/of/lib/modules" in my
.profile or whatever in order to drop the modules into another directory
at "make modules_install" time.  Is this one of the things folks are
talking about?

Hal Duston

Re: several messages

[Bill Davidsen]

On Wed, 22 Jan 2003, David Woodhouse wrote:

> 
> davidsen@tmr.com said:
> >  `uname -r` is the kernel version of the running kernel. It is NOT by
> > magic the kernel version of the kernel you are building... 
> 
> Er, yes. And what's your point? 
> 
> There is _no_ magic that will find the kernel you want to build against
> today without any input from you. Using the build tree for the
> currently-running kernel, if installed in the standard place, is as good a
> default as any. Of course you should be permitted to override that default.

You make my point for me, there is no magic, and when building a module it
should require that the directory be specified by either a command line
option (as noted below) or by being built as part of a source tree. There
*is* no good default in that particular case.


On Wed, 22 Jan 2003, Hal Duston wrote:

> I use "INSTALL_MOD_PATH=put/the/modules/here/instead/of/lib/modules" in my
> .profile or whatever in order to drop the modules into another directory
> at "make modules_install" time.  Is this one of the things folks are
> talking about?

Related for sure, the point I was making was that there is no good default
place to put modules built outside a kernel source tree (and probably also
when built for multiple kernels). I was suggesting that the module tree of
the running kernel might be a really poor choice. I don't think I was
clear in my first post, I was not suggesting a better default, I was
suggesting that any default is likely to bite.

I'm not unhappy that Mr. Woodhouse disagrees, I just think he missed my
point the first time and I'm trying to clarify.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: several messages

[David Woodhouse]

davidsen@tmr.com said:
>  You make my point for me, there is no magic, and when building a
> module it should require that the directory be specified by either a
> command line option (as noted below) or by being built as part of a
> source tree. There *is* no good default in that particular case.

/lib/modules/`uname -r`/build _is_ a good default for a module to build 
again. It is correct in more cases than a simple failure to do anything.

For _installing_, the correct place to install the built objects is surely
/lib/modules/$(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION) where the 
variables are obtained from the top-level Makefile in the kernel sources 
you built against.

You _default_ to building and installing for the current kernel, if it's
installed properly. But of course you should be permitted to override that 
on the command line.


> Related for sure, the point I was making was that there is no good
> default place to put modules built outside a kernel source tree (and
> probably also when built for multiple kernels).

I disagree. Modutils will look in only one place -- the /lib/modules/... 
directory corresponding to the kernel version for which you built each 
module. Each module, therefore, should go into the directory corresponding 
to the version of the kernel against which it was built.

Finding the appropriate _installation_ directory is trivial, surely? You 
can even find it from the 'kernel_version' stamp _inside_ the object file, 
without any other information?

--
dwmw2