LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch 00/13] s390 bux fixes for 2.6.25-rc2.
@ 2008-02-19 14:40 Martin Schwidefsky
  2008-02-19 14:40 ` [patch 01/13] cio: Remember to initialize recovery_lock Martin Schwidefsky
                   ` (12 more replies)
  0 siblings, 13 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390

Greetings,
this and that for s390. The shortlog:

Cornelia Huck (2):
      [S390] cio: Remember to initialize recovery_lock.
      [S390] cio: Do timed recovery on workqueue.

Heiko Carstens (6):
      [S390] Let NR_CPUS default to 32/64 on s390/s390x.
      [S390] Make sure enabled wait psw is loaded in default_idle.
      [S390] Initialize per cpu lowcores on cpu hotplug.
      [S390] qdio: fix qdio_activate timeout handling.
      [S390] etr: fix compile error on !SMP
      [S390] Fix futex_atomic_cmpxchg_std inline assembly.

Martin Schwidefsky (1):
      [S390] find bit corner case.

Peter Oberparleiter (1):
      [S390] sclp: clean up send/receive naming scheme

Roel Kluin (1):
      [S390] dcss: Fix Unlikely(x) != y

Stefan Weinhuber (1):
      [S390] dasd: fix locking in __dasd_device_process_final_queue

Ursula Braun (1):
      [S390] qdio: FCP/SCSI write I/O stagnates on LPAR

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 01/13] cio: Remember to initialize recovery_lock.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 02/13] cio: Do timed recovery on workqueue Martin Schwidefsky
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Cornelia Huck, Martin Schwidefsky

[-- Attachment #1: 001-cio-lockinit.diff --]
[-- Type: text/plain, Size: 776 bytes --]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/s390/cio/device.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: quilt-2.6/drivers/s390/cio/device.c
===================================================================
--- quilt-2.6.orig/drivers/s390/cio/device.c
+++ quilt-2.6/drivers/s390/cio/device.c
@@ -32,7 +32,7 @@
 #include "io_sch.h"
 
 static struct timer_list recovery_timer;
-static spinlock_t recovery_lock;
+static DEFINE_SPINLOCK(recovery_lock);
 static int recovery_phase;
 static const unsigned long recovery_delay[] = { 3, 30, 300 };
 

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 02/13] cio: Do timed recovery on workqueue.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
  2008-02-19 14:40 ` [patch 01/13] cio: Remember to initialize recovery_lock Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 03/13] Let NR_CPUS default to 32/64 on s390/s390x Martin Schwidefsky
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Cornelia Huck, Martin Schwidefsky

[-- Attachment #1: 002-cio-recovery.diff --]
[-- Type: text/plain, Size: 1278 bytes --]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

We can't do our recovery in softirq context, so we schedule it from
our timer function.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/s390/cio/device.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Index: quilt-2.6/drivers/s390/cio/device.c
===================================================================
--- quilt-2.6.orig/drivers/s390/cio/device.c
+++ quilt-2.6/drivers/s390/cio/device.c
@@ -1535,7 +1535,7 @@ static int recovery_check(struct device 
 	return 0;
 }
 
-static void recovery_func(unsigned long data)
+static void recovery_work_func(struct work_struct *unused)
 {
 	int redo = 0;
 
@@ -1553,6 +1553,17 @@ static void recovery_func(unsigned long 
 		CIO_MSG_EVENT(2, "recovery: end\n");
 }
 
+static DECLARE_WORK(recovery_work, recovery_work_func);
+
+static void recovery_func(unsigned long data)
+{
+	/*
+	 * We can't do our recovery in softirq context and it's not
+	 * performance critical, so we schedule it.
+	 */
+	schedule_work(&recovery_work);
+}
+
 void ccw_device_schedule_recovery(void)
 {
 	unsigned long flags;

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 03/13] Let NR_CPUS default to 32/64 on s390/s390x.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
  2008-02-19 14:40 ` [patch 01/13] cio: Remember to initialize recovery_lock Martin Schwidefsky
  2008-02-19 14:40 ` [patch 02/13] cio: Do timed recovery on workqueue Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 04/13] Make sure enabled wait psw is loaded in default_idle Martin Schwidefsky
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Heiko Carstens, Martin Schwidefsky

[-- Attachment #1: 003-nrcpus.diff --]
[-- Type: text/plain, Size: 811 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 arch/s390/Kconfig |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: quilt-2.6/arch/s390/Kconfig
===================================================================
--- quilt-2.6.orig/arch/s390/Kconfig
+++ quilt-2.6/arch/s390/Kconfig
@@ -100,7 +100,8 @@ config NR_CPUS
 	int "Maximum number of CPUs (2-64)"
 	range 2 64
 	depends on SMP
-	default "32"
+	default "32" if !64BIT
+	default "64" if 64BIT
 	help
 	  This allows you to specify the maximum number of CPUs which this
 	  kernel will support.  The maximum supported value is 64 and the

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 04/13] Make sure enabled wait psw is loaded in default_idle.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (2 preceding siblings ...)
  2008-02-19 14:40 ` [patch 03/13] Let NR_CPUS default to 32/64 on s390/s390x Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 05/13] dasd: fix locking in __dasd_device_process_final_queue Martin Schwidefsky
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Heiko Carstens, Martin Schwidefsky

[-- Attachment #1: 004-idle.diff --]
[-- Type: text/plain, Size: 1760 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

If both NO_IDLE_HZ and VIRT_TIMER are disabled default_idle won't load
an enabled wait psw and busy loop instead. This is because the
idle_chain is empty and the return value of atomic_notifier_call_chain
will be NOTIFY_DONE, which causes default_idle to return instead of
loading an enabled wait psw.
Fix this by calling __atomic_notifier_call_chain instead and add proper
return value handling.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 arch/s390/kernel/process.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Index: quilt-2.6/arch/s390/kernel/process.c
===================================================================
--- quilt-2.6.orig/arch/s390/kernel/process.c
+++ quilt-2.6/arch/s390/kernel/process.c
@@ -114,24 +114,27 @@ extern void s390_handle_mcck(void);
 static void default_idle(void)
 {
 	int cpu, rc;
+	int nr_calls = 0;
+	void *hcpu;
 #ifdef CONFIG_SMP
 	struct s390_idle_data *idle;
 #endif
 
 	/* CPU is going idle. */
 	cpu = smp_processor_id();
-
+	hcpu = (void *)(long)cpu;
 	local_irq_disable();
 	if (need_resched()) {
 		local_irq_enable();
 		return;
 	}
 
-	rc = atomic_notifier_call_chain(&idle_chain,
-					S390_CPU_IDLE, (void *)(long) cpu);
-	if (rc != NOTIFY_OK && rc != NOTIFY_DONE)
-		BUG();
-	if (rc != NOTIFY_OK) {
+	rc = __atomic_notifier_call_chain(&idle_chain, S390_CPU_IDLE, hcpu, -1,
+					  &nr_calls);
+	if (rc == NOTIFY_BAD) {
+		nr_calls--;
+		__atomic_notifier_call_chain(&idle_chain, S390_CPU_NOT_IDLE,
+					     hcpu, nr_calls, NULL);
 		local_irq_enable();
 		return;
 	}

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 05/13] dasd: fix locking in __dasd_device_process_final_queue
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (3 preceding siblings ...)
  2008-02-19 14:40 ` [patch 04/13] Make sure enabled wait psw is loaded in default_idle Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 06/13] find bit corner case Martin Schwidefsky
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Stefan Weinhuber, Martin Schwidefsky

[-- Attachment #1: 005-dasd-locking.diff --]
[-- Type: text/plain, Size: 1947 bytes --]

From: Stefan Weinhuber <wein@de.ibm.com>

After setting the status of the cqr and releasing the lock for the
block cqr queue, we call the cqr callback function, which will usually
just trigger the dasd_block_tasklet. But when the tasklet is already
running the cqr might be processed before we invoke the callback
function. In rare cases the callback pointer may already be invalid
by the time we want to call it, which will result in a panic.
Solution: Call the callback function first and then release the lock.

Signed-off-by: Stefan Weinhuber <wein@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/s390/block/dasd.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Index: quilt-2.6/drivers/s390/block/dasd.c
===================================================================
--- quilt-2.6.orig/drivers/s390/block/dasd.c
+++ quilt-2.6/drivers/s390/block/dasd.c
@@ -1149,12 +1149,14 @@ static void __dasd_device_process_final_
 {
 	struct list_head *l, *n;
 	struct dasd_ccw_req *cqr;
+	struct dasd_block *block;
 
 	list_for_each_safe(l, n, final_queue) {
 		cqr = list_entry(l, struct dasd_ccw_req, devlist);
 		list_del_init(&cqr->devlist);
-		if (cqr->block)
-			spin_lock_bh(&cqr->block->queue_lock);
+		block = cqr->block;
+		if (block)
+			spin_lock_bh(&block->queue_lock);
 		switch (cqr->status) {
 		case DASD_CQR_SUCCESS:
 			cqr->status = DASD_CQR_DONE;
@@ -1172,15 +1174,13 @@ static void __dasd_device_process_final_
 				    cqr, cqr->status);
 			BUG();
 		}
-		if (cqr->block)
-			spin_unlock_bh(&cqr->block->queue_lock);
 		if (cqr->callback != NULL)
 			(cqr->callback)(cqr, cqr->callback_data);
+		if (block)
+			spin_unlock_bh(&block->queue_lock);
 	}
 }
 
-
-
 /*
  * Take a look at the first request on the ccw queue and check
  * if it reached its expire time. If so, terminate the IO.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 06/13] find bit corner case.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (4 preceding siblings ...)
  2008-02-19 14:40 ` [patch 05/13] dasd: fix locking in __dasd_device_process_final_queue Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 07/13] Initialize per cpu lowcores on cpu hotplug Martin Schwidefsky
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Martin Schwidefsky

[-- Attachment #1: 006-bitops.diff --]
[-- Type: text/plain, Size: 1530 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Fix [ext2_]find_first_[zero_]bit for the corner case of an all clear
or all set bit field by always handling that last word of the bit field
with __ffz_word/__ffs_word.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 include/asm-s390/bitops.h |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Index: quilt-2.6/include/asm-s390/bitops.h
===================================================================
--- quilt-2.6.orig/include/asm-s390/bitops.h
+++ quilt-2.6/include/asm-s390/bitops.h
@@ -456,16 +456,18 @@ static inline unsigned long __ffz_word_l
 
 	asm volatile(
 #ifndef __s390x__
-		"	ahi	%1,31\n"
-		"	srl	%1,5\n"
+		"	ahi	%1,-1\n"
+		"	sra	%1,5\n"
+		"	jz	1f\n"
 		"0:	c	%2,0(%0,%3)\n"
 		"	jne	1f\n"
 		"	la	%0,4(%0)\n"
 		"	brct	%1,0b\n"
 		"1:\n"
 #else
-		"	aghi	%1,63\n"
-		"	srlg	%1,%1,6\n"
+		"	aghi	%1,-1\n"
+		"	srag	%1,%1,6\n"
+		"	jz	1f\n"
 		"0:	cg	%2,0(%0,%3)\n"
 		"	jne	1f\n"
 		"	la	%0,8(%0)\n"
@@ -491,16 +493,18 @@ static inline unsigned long __ffs_word_l
 
 	asm volatile(
 #ifndef __s390x__
-		"	ahi	%1,31\n"
-		"	srl	%1,5\n"
+		"	ahi	%1,-1\n"
+		"	sra	%1,5\n"
+		"	jz	1f\n"
 		"0:	c	%2,0(%0,%3)\n"
 		"	jne	1f\n"
 		"	la	%0,4(%0)\n"
 		"	brct	%1,0b\n"
 		"1:\n"
 #else
-		"	aghi	%1,63\n"
-		"	srlg	%1,%1,6\n"
+		"	aghi	%1,-1\n"
+		"	srag	%1,%1,6\n"
+		"	jz	1f\n"
 		"0:	cg	%2,0(%0,%3)\n"
 		"	jne	1f\n"
 		"	la	%0,8(%0)\n"

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (5 preceding siblings ...)
  2008-02-19 14:40 ` [patch 06/13] find bit corner case Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 15:13   ` Bastian Blank
  2008-02-19 14:40 ` [patch 08/13] qdio: fix qdio_activate timeout handling Martin Schwidefsky
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Heiko Carstens, Martin Schwidefsky

[-- Attachment #1: 007-lowcore.diff --]
[-- Type: text/plain, Size: 4123 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Just copy the first 512 read-only bytes of the current cpu lowcore if
a new cpu gets onlined. The rest is zeroed out and must be explicitly
initialized. Current code just copies the entire lowcore and
initializes the needed fields.
This should reveal bugs in future enhancements quite early.
Also when the lowcore of the first cpu is replaced this is now done
atomically (no interrupts, no machine checks).

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 arch/s390/kernel/smp.c |   53 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 15 deletions(-)

Index: quilt-2.6/arch/s390/kernel/smp.c
===================================================================
--- quilt-2.6.orig/arch/s390/kernel/smp.c
+++ quilt-2.6/arch/s390/kernel/smp.c
@@ -626,13 +626,17 @@ static int __cpuinit smp_alloc_lowcore(i
 	if (!lowcore)
 		return -ENOMEM;
 	async_stack = __get_free_pages(GFP_KERNEL, ASYNC_ORDER);
-	if (!async_stack)
-		goto out_async_stack;
 	panic_stack = __get_free_page(GFP_KERNEL);
-	if (!panic_stack)
-		goto out_panic_stack;
-
-	*lowcore = S390_lowcore;
+	if (!panic_stack || !async_stack)
+		goto out;
+	/*
+	 * Only need to copy the first 512 bytes from address 0. But since
+	 * the compiler emits a warning if src == NULL for memcpy use copy_page
+	 * instead. Copies more than needed but this code is not performance
+	 * critical.
+	 */
+	copy_page(lowcore, &S390_lowcore);
+	memset((void *)lowcore + 512, 0, sizeof(*lowcore) - 512);
 	lowcore->async_stack = async_stack + ASYNC_SIZE;
 	lowcore->panic_stack = panic_stack + PAGE_SIZE;
 
@@ -653,9 +657,8 @@ static int __cpuinit smp_alloc_lowcore(i
 out_save_area:
 	free_page(panic_stack);
 #endif
-out_panic_stack:
+out:
 	free_pages(async_stack, ASYNC_ORDER);
-out_async_stack:
 	free_pages((unsigned long) lowcore, lc_order);
 	return -ENOMEM;
 }
@@ -719,8 +722,8 @@ int __cpuinit __cpu_up(unsigned int cpu)
 	cpu_lowcore->percpu_offset = __per_cpu_offset[cpu];
 	cpu_lowcore->current_task = (unsigned long) idle;
 	cpu_lowcore->cpu_data.cpu_nr = cpu;
-	cpu_lowcore->softirq_pending = 0;
-	cpu_lowcore->ext_call_fast = 0;
+	cpu_lowcore->kernel_asce = S390_lowcore.kernel_asce;
+	cpu_lowcore->ipl_device = S390_lowcore.ipl_device;
 	eieio();
 
 	while (signal_processor(cpu, sigp_restart) == sigp_busy)
@@ -797,23 +800,43 @@ void cpu_die(void)
 
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
+#ifndef CONFIG_64BIT
+	unsigned long save_area = 0;
+#endif
+	unsigned long async_stack, panic_stack;
+	struct _lowcore *lowcore;
 	unsigned int cpu;
+	int lc_order;
 
 	smp_detect_cpus();
 
 	/* request the 0x1201 emergency signal external interrupt */
 	if (register_external_interrupt(0x1201, do_ext_call_interrupt) != 0)
 		panic("Couldn't request external interrupt 0x1201");
-	memset(lowcore_ptr, 0, sizeof(lowcore_ptr));
 	print_cpu_info(&S390_lowcore.cpu_data);
-	smp_alloc_lowcore(smp_processor_id());
 
+	/* Reallocate current lowcore, but keep its contents. */
+	lc_order = sizeof(long) == 8 ? 1 : 0;
+	lowcore = (void *) __get_free_pages(GFP_KERNEL | GFP_DMA, lc_order);
+	panic_stack = __get_free_page(GFP_KERNEL);
+	async_stack = __get_free_pages(GFP_KERNEL, ASYNC_ORDER);
 #ifndef CONFIG_64BIT
 	if (MACHINE_HAS_IEEE)
-		ctl_set_bit(14, 29); /* enable extended save area */
+		save_area = get_zeroed_page(GFP_KERNEL);
 #endif
-	set_prefix((u32)(unsigned long) lowcore_ptr[smp_processor_id()]);
-
+	local_irq_disable();
+	local_mcck_disable();
+	lowcore_ptr[smp_processor_id()] = lowcore;
+	*lowcore = S390_lowcore;
+	lowcore->panic_stack = panic_stack + PAGE_SIZE;
+	lowcore->async_stack = async_stack + ASYNC_SIZE;
+#ifndef CONFIG_64BIT
+	if (MACHINE_HAS_IEEE)
+		lowcore->extended_save_area_addr = (u32) save_area;
+#endif
+	set_prefix((u32)(unsigned long) lowcore);
+	local_mcck_enable();
+	local_irq_enable();
 	for_each_possible_cpu(cpu)
 		if (cpu != smp_processor_id())
 			smp_create_idle(cpu);

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 08/13] qdio: fix qdio_activate timeout handling.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (6 preceding siblings ...)
  2008-02-19 14:40 ` [patch 07/13] Initialize per cpu lowcores on cpu hotplug Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 09/13] etr: fix compile error on !SMP Martin Schwidefsky
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390
  Cc: Utz Bacher, Jan Glauber, Ursula Braun, Martin Peschke,
	Cornelia Huck, Heiko Carstens, Martin Schwidefsky

[-- Attachment #1: 008-qdio-timeout.diff --]
[-- Type: text/plain, Size: 2407 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Current code in qdio_activate waits for at least 5 seconds
until it returns. It may return earlier if an error occurs,
but not if everything is ok. This large timeout value
became visible with commit dfa77f611ff295598e218aa0eb6efa73a5cf26d0
"qdio: set QDIO_ACTIVATE_TIMEOUT to 5s", which intended to
fix the timeout value which was zero. In turn setting an
FCP adapter online took 5 seconds.

In practice waiting for 5ms before continuing is sufficient
as pointed out by Utz Bacher and Cornelia Huck.

Cc: Utz Bacher <utz.bacher@de.ibm.com>
Cc: Jan Glauber <jan.glauber@de.ibm.com>
Cc: Ursula Braun <braunu@de.ibm.com>
Cc: Martin Peschke <mp3@de.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/s390/cio/qdio.c |   10 ++--------
 drivers/s390/cio/qdio.h |    2 +-
 2 files changed, 3 insertions(+), 9 deletions(-)

Index: quilt-2.6/drivers/s390/cio/qdio.c
===================================================================
--- quilt-2.6.orig/drivers/s390/cio/qdio.c
+++ quilt-2.6/drivers/s390/cio/qdio.c
@@ -32,7 +32,7 @@
 
 #include <linux/module.h>
 #include <linux/init.h>
-
+#include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/proc_fs.h>
@@ -3332,13 +3332,7 @@ qdio_activate(struct ccw_device *cdev, i
 		}
 	}
 
-	wait_event_interruptible_timeout(cdev->private->wait_q,
-					 ((irq_ptr->state ==
-					  QDIO_IRQ_STATE_STOPPED) ||
-					  (irq_ptr->state ==
-					   QDIO_IRQ_STATE_ERR)),
-					 QDIO_ACTIVATE_TIMEOUT);
-
+	msleep(QDIO_ACTIVATE_TIMEOUT);
 	switch (irq_ptr->state) {
 	case QDIO_IRQ_STATE_STOPPED:
 	case QDIO_IRQ_STATE_ERR:
Index: quilt-2.6/drivers/s390/cio/qdio.h
===================================================================
--- quilt-2.6.orig/drivers/s390/cio/qdio.h
+++ quilt-2.6/drivers/s390/cio/qdio.h
@@ -57,10 +57,10 @@
 					    of the queue to 0 */
 
 #define QDIO_ESTABLISH_TIMEOUT (1*HZ)
-#define QDIO_ACTIVATE_TIMEOUT (5*HZ)
 #define QDIO_CLEANUP_CLEAR_TIMEOUT (20*HZ)
 #define QDIO_CLEANUP_HALT_TIMEOUT (10*HZ)
 #define QDIO_FORCE_CHECK_TIMEOUT (10*HZ)
+#define QDIO_ACTIVATE_TIMEOUT (5) /* 5 ms */
 
 enum qdio_irq_states {
 	QDIO_IRQ_STATE_INACTIVE,

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 09/13] etr: fix compile error on !SMP
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (7 preceding siblings ...)
  2008-02-19 14:40 ` [patch 08/13] qdio: fix qdio_activate timeout handling Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 10/13] sclp: clean up send/receive naming scheme Martin Schwidefsky
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Heiko Carstens, Martin Schwidefsky

[-- Attachment #1: 009-etr-nonsmp.diff --]
[-- Type: text/plain, Size: 1400 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Since a5fbb6d1064be885d2a6b82f625186753cf74848
"KVM: fix !SMP build error" smp_call_function isn't a define anymore
that folds into nothing but a define that calls up_smp_call_function
with all parameters. Hence we cannot #ifdef out the unused code
anymore...
This seems to be the preferred method, so do this for s390 as well.

arch/s390/kernel/time.c: In function 'etr_sync_clock':
arch/s390/kernel/time.c:825: error: 'clock_sync_cpu_start' undeclared
arch/s390/kernel/time.c:862: error: 'clock_sync_cpu_end' undeclared

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 arch/s390/kernel/time.c |    2 --
 1 file changed, 2 deletions(-)

Index: quilt-2.6/arch/s390/kernel/time.c
===================================================================
--- quilt-2.6.orig/arch/s390/kernel/time.c
+++ quilt-2.6/arch/s390/kernel/time.c
@@ -744,7 +744,6 @@ static void etr_adjust_time(unsigned lon
 	}
 }
 
-#ifdef CONFIG_SMP
 static void etr_sync_cpu_start(void *dummy)
 {
 	int *in_sync = dummy;
@@ -777,7 +776,6 @@ static void etr_sync_cpu_start(void *dum
 static void etr_sync_cpu_end(void *dummy)
 {
 }
-#endif /* CONFIG_SMP */
 
 /*
  * Sync the TOD clock using the port refered to by aibp. This port

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 10/13] sclp: clean up send/receive naming scheme
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (8 preceding siblings ...)
  2008-02-19 14:40 ` [patch 09/13] etr: fix compile error on !SMP Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 11/13] dcss: Fix Unlikely(x) != y Martin Schwidefsky
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Peter Oberparleiter, Martin Schwidefsky

[-- Attachment #1: 010-sclp-cleanup.diff --]
[-- Type: text/plain, Size: 5757 bytes --]

From: Peter Oberparleiter <peter.oberparleiter@de.ibm.com>

Make state change events adjust the correct mask by cleaning up
naming inconsistencies. Also remove chance for lockup by removing
unnecessary mask related check before reading events.

Signed-off-by: Peter Oberparleiter <peter.oberparleiter@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/s390/char/sclp.c         |   12 ++++++------
 drivers/s390/char/sclp.h         |    6 ++++--
 drivers/s390/char/sclp_config.c  |    2 +-
 drivers/s390/char/sclp_cpi_sys.c |    2 +-
 drivers/s390/char/sclp_rw.c      |    4 ++--
 drivers/s390/char/sclp_vt220.c   |    2 +-
 6 files changed, 15 insertions(+), 13 deletions(-)

Index: quilt-2.6/drivers/s390/char/sclp.c
===================================================================
--- quilt-2.6.orig/drivers/s390/char/sclp.c
+++ quilt-2.6/drivers/s390/char/sclp.c
@@ -29,10 +29,10 @@ static ext_int_info_t ext_int_info_hwc;
 /* Lock to protect internal data consistency. */
 static DEFINE_SPINLOCK(sclp_lock);
 
-/* Mask of events that we can receive from the sclp interface. */
+/* Mask of events that we can send to the sclp interface. */
 static sccb_mask_t sclp_receive_mask;
 
-/* Mask of events that we can send to the sclp interface. */
+/* Mask of events that we can receive from the sclp interface. */
 static sccb_mask_t sclp_send_mask;
 
 /* List of registered event listeners and senders. */
@@ -380,7 +380,7 @@ sclp_interrupt_handler(__u16 code)
 		}
 		sclp_running_state = sclp_running_state_idle;
 	}
-	if (evbuf_pending && sclp_receive_mask != 0 &&
+	if (evbuf_pending &&
 	    sclp_activation_state == sclp_activation_state_active)
 		__sclp_queue_read_req();
 	spin_unlock(&sclp_lock);
@@ -459,8 +459,8 @@ sclp_dispatch_state_change(void)
 		reg = NULL;
 		list_for_each(l, &sclp_reg_list) {
 			reg = list_entry(l, struct sclp_register, list);
-			receive_mask = reg->receive_mask & sclp_receive_mask;
-			send_mask = reg->send_mask & sclp_send_mask;
+			receive_mask = reg->send_mask & sclp_receive_mask;
+			send_mask = reg->receive_mask & sclp_send_mask;
 			if (reg->sclp_receive_mask != receive_mask ||
 			    reg->sclp_send_mask != send_mask) {
 				reg->sclp_receive_mask = receive_mask;
@@ -615,8 +615,8 @@ struct init_sccb {
 	u16 mask_length;
 	sccb_mask_t receive_mask;
 	sccb_mask_t send_mask;
-	sccb_mask_t sclp_send_mask;
 	sccb_mask_t sclp_receive_mask;
+	sccb_mask_t sclp_send_mask;
 } __attribute__((packed));
 
 /* Prepare init mask request. Called while sclp_lock is locked. */
Index: quilt-2.6/drivers/s390/char/sclp_config.c
===================================================================
--- quilt-2.6.orig/drivers/s390/char/sclp_config.c
+++ quilt-2.6/drivers/s390/char/sclp_config.c
@@ -64,7 +64,7 @@ static int __init sclp_conf_init(void)
 		return rc;
 	}
 
-	if (!(sclp_conf_register.sclp_receive_mask & EVTYP_CONFMGMDATA_MASK)) {
+	if (!(sclp_conf_register.sclp_send_mask & EVTYP_CONFMGMDATA_MASK)) {
 		printk(KERN_WARNING TAG "no configuration management.\n");
 		sclp_unregister(&sclp_conf_register);
 		rc = -ENOSYS;
Index: quilt-2.6/drivers/s390/char/sclp_cpi_sys.c
===================================================================
--- quilt-2.6.orig/drivers/s390/char/sclp_cpi_sys.c
+++ quilt-2.6/drivers/s390/char/sclp_cpi_sys.c
@@ -129,7 +129,7 @@ static int cpi_req(void)
 			"to hardware console.\n");
 		goto out;
 	}
-	if (!(sclp_cpi_event.sclp_send_mask & EVTYP_CTLPROGIDENT_MASK)) {
+	if (!(sclp_cpi_event.sclp_receive_mask & EVTYP_CTLPROGIDENT_MASK)) {
 		printk(KERN_WARNING "cpi: no control program "
 			"identification support\n");
 		rc = -EOPNOTSUPP;
Index: quilt-2.6/drivers/s390/char/sclp.h
===================================================================
--- quilt-2.6.orig/drivers/s390/char/sclp.h
+++ quilt-2.6/drivers/s390/char/sclp.h
@@ -122,11 +122,13 @@ struct sclp_req {
 /* of some routines it wants to be called from the low level driver */
 struct sclp_register {
 	struct list_head list;
-	/* event masks this user is registered for */
+	/* User wants to receive: */
 	sccb_mask_t receive_mask;
+	/* User wants to send: */
 	sccb_mask_t send_mask;
-	/* actually present events */
+	/* H/W can receive: */
 	sccb_mask_t sclp_receive_mask;
+	/* H/W can send: */
 	sccb_mask_t sclp_send_mask;
 	/* called if event type availability changes */
 	void (*state_change_fn)(struct sclp_register *);
Index: quilt-2.6/drivers/s390/char/sclp_rw.c
===================================================================
--- quilt-2.6.orig/drivers/s390/char/sclp_rw.c
+++ quilt-2.6/drivers/s390/char/sclp_rw.c
@@ -452,10 +452,10 @@ sclp_emit_buffer(struct sclp_buffer *buf
 		return -EIO;
 
 	sccb = buffer->sccb;
-	if (sclp_rw_event.sclp_send_mask & EVTYP_MSG_MASK)
+	if (sclp_rw_event.sclp_receive_mask & EVTYP_MSG_MASK)
 		/* Use normal write message */
 		sccb->msg_buf.header.type = EVTYP_MSG;
-	else if (sclp_rw_event.sclp_send_mask & EVTYP_PMSGCMD_MASK)
+	else if (sclp_rw_event.sclp_receive_mask & EVTYP_PMSGCMD_MASK)
 		/* Use write priority message */
 		sccb->msg_buf.header.type = EVTYP_PMSGCMD;
 	else
Index: quilt-2.6/drivers/s390/char/sclp_vt220.c
===================================================================
--- quilt-2.6.orig/drivers/s390/char/sclp_vt220.c
+++ quilt-2.6/drivers/s390/char/sclp_vt220.c
@@ -202,7 +202,7 @@ sclp_vt220_callback(struct sclp_req *req
 static int
 __sclp_vt220_emit(struct sclp_vt220_request *request)
 {
-	if (!(sclp_vt220_register.sclp_send_mask & EVTYP_VT220MSG_MASK)) {
+	if (!(sclp_vt220_register.sclp_receive_mask & EVTYP_VT220MSG_MASK)) {
 		request->sclp_req.status = SCLP_REQ_FAILED;
 		return -EIO;
 	}

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 11/13] dcss: Fix Unlikely(x) != y
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (9 preceding siblings ...)
  2008-02-19 14:40 ` [patch 10/13] sclp: clean up send/receive naming scheme Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:40 ` [patch 12/13] Fix futex_atomic_cmpxchg_std inline assembly Martin Schwidefsky
  2008-02-19 14:41 ` [patch 13/13] qdio: FCP/SCSI write I/O stagnates on LPAR Martin Schwidefsky
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390
  Cc: Gerald Schaefer, Stefan Weinhuber, Carsten Otte, Roel Kluin,
	Heiko Carstens, Martin Schwidefsky

[-- Attachment #1: 011-dcssblk.diff --]
[-- Type: text/plain, Size: 1132 bytes --]

From: Roel Kluin <12o3l@tiscali.nl>

Fix Unlikely(x) != y

Cc: Gerald Schaefer <geraldsc@de.ibm.com>
Cc: Stefan Weinhuber <wein@de.ibm.com>
Cc: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/s390/block/dcssblk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: quilt-2.6/drivers/s390/block/dcssblk.c
===================================================================
--- quilt-2.6.orig/drivers/s390/block/dcssblk.c
+++ quilt-2.6/drivers/s390/block/dcssblk.c
@@ -666,7 +666,7 @@ dcssblk_make_request(struct request_queu
 		page_addr = (unsigned long)
 			page_address(bvec->bv_page) + bvec->bv_offset;
 		source_addr = dev_info->start + (index<<12) + bytes_done;
-		if (unlikely(page_addr & 4095) != 0 || (bvec->bv_len & 4095) != 0)
+		if (unlikely((page_addr & 4095) != 0) || (bvec->bv_len & 4095) != 0)
 			// More paranoia.
 			goto fail;
 		if (bio_data_dir(bio) == READ) {

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 12/13] Fix futex_atomic_cmpxchg_std inline assembly.
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (10 preceding siblings ...)
  2008-02-19 14:40 ` [patch 11/13] dcss: Fix Unlikely(x) != y Martin Schwidefsky
@ 2008-02-19 14:40 ` Martin Schwidefsky
  2008-02-19 14:41 ` [patch 13/13] qdio: FCP/SCSI write I/O stagnates on LPAR Martin Schwidefsky
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:40 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: stable, Heiko Carstens, Martin Schwidefsky

[-- Attachment #1: 012-futex-extable.diff --]
[-- Type: text/plain, Size: 1229 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Add missing exception table entry so that the kernel can handle
proctection exceptions as well on the cs instruction. Currently only
specification exceptions are handled correctly.
The missing entry allows user space to crash the kernel.

Cc: stable <stable@kernel.org>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 arch/s390/lib/uaccess_std.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Index: quilt-2.6/arch/s390/lib/uaccess_std.c
===================================================================
--- quilt-2.6.orig/arch/s390/lib/uaccess_std.c
+++ quilt-2.6/arch/s390/lib/uaccess_std.c
@@ -293,10 +293,10 @@ int futex_atomic_cmpxchg_std(int __user 
 
 	asm volatile(
 		"   sacf 256\n"
-		"   cs   %1,%4,0(%5)\n"
-		"0: lr   %0,%1\n"
-		"1: sacf 0\n"
-		EX_TABLE(0b,1b)
+		"0: cs   %1,%4,0(%5)\n"
+		"1: lr   %0,%1\n"
+		"2: sacf 0\n"
+		EX_TABLE(0b,2b) EX_TABLE(1b,2b)
 		: "=d" (ret), "+d" (oldval), "=m" (*uaddr)
 		: "0" (-EFAULT), "d" (newval), "a" (uaddr), "m" (*uaddr)
 		: "cc", "memory" );

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch 13/13] qdio: FCP/SCSI write I/O stagnates on LPAR
  2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
                   ` (11 preceding siblings ...)
  2008-02-19 14:40 ` [patch 12/13] Fix futex_atomic_cmpxchg_std inline assembly Martin Schwidefsky
@ 2008-02-19 14:41 ` Martin Schwidefsky
  12 siblings, 0 replies; 21+ messages in thread
From: Martin Schwidefsky @ 2008-02-19 14:41 UTC (permalink / raw)
  To: linux-kernel, linux-s390; +Cc: Ursula Braun, Martin Schwidefsky

[-- Attachment #1: 013-qdio-stalls.diff --]
[-- Type: text/plain, Size: 940 bytes --]

From: Ursula Braun <braunu@de.ibm.com>

If running on LPAR, qdio might overlook an incoming buffer in certain
scenarios. The patch makes sure that incoming buffers are detected
immediately in all situations.

Signed-off-by: Ursula Braun <braunu@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 drivers/s390/cio/qdio.c |    3 ---
 1 file changed, 3 deletions(-)

Index: quilt-2.6/drivers/s390/cio/qdio.c
===================================================================
--- quilt-2.6.orig/drivers/s390/cio/qdio.c
+++ quilt-2.6/drivers/s390/cio/qdio.c
@@ -1215,9 +1215,6 @@ tiqdio_is_inbound_q_done(struct qdio_q *
 
 	if (!no_used)
 		return 1;
-	if (!q->siga_sync && !irq->is_qebsm)
-		/* we'll check for more primed buffers in qeth_stop_polling */
-		return 0;
 	if (irq->is_qebsm) {
 		count = 1;
 		start_buf = q->first_to_check;

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-19 14:40 ` [patch 07/13] Initialize per cpu lowcores on cpu hotplug Martin Schwidefsky
@ 2008-02-19 15:13   ` Bastian Blank
  2008-02-19 15:38     ` Heiko Carstens
  0 siblings, 1 reply; 21+ messages in thread
From: Bastian Blank @ 2008-02-19 15:13 UTC (permalink / raw)
  To: Martin Schwidefsky; +Cc: linux-kernel, linux-s390, Heiko Carstens

On Tue, Feb 19, 2008 at 03:40:54PM +0100, Martin Schwidefsky wrote:
> +	/*
> +	 * Only need to copy the first 512 bytes from address 0. But since
> +	 * the compiler emits a warning if src == NULL for memcpy use copy_page
> +	 * instead. Copies more than needed but this code is not performance
> +	 * critical.
> +	 */
> +	copy_page(lowcore, &S390_lowcore);

Boah, workaround alert. Why do you not fix the compiler?

Bastian

-- 
We fight only when there is no other choice.  We prefer the ways of
peaceful contact.
		-- Kirk, "Spectre of the Gun", stardate 4385.3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-19 15:13   ` Bastian Blank
@ 2008-02-19 15:38     ` Heiko Carstens
  2008-02-19 15:41       ` Heiko Carstens
  0 siblings, 1 reply; 21+ messages in thread
From: Heiko Carstens @ 2008-02-19 15:38 UTC (permalink / raw)
  To: Bastian Blank, Martin Schwidefsky, linux-kernel, linux-s390

On Tue, Feb 19, 2008 at 04:13:55PM +0100, Bastian Blank wrote:
> On Tue, Feb 19, 2008 at 03:40:54PM +0100, Martin Schwidefsky wrote:
> > +	/*
> > +	 * Only need to copy the first 512 bytes from address 0. But since
> > +	 * the compiler emits a warning if src == NULL for memcpy use copy_page
> > +	 * instead. Copies more than needed but this code is not performance
> > +	 * critical.
> > +	 */
> > +	copy_page(lowcore, &S390_lowcore);
> 
> Boah, workaround alert. Why do you not fix the compiler?

We need to copy from address 0 (that's where the lowcore resides). But gcc
insists to complain if memcpy is used with src == NULL.. Now what?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-19 15:38     ` Heiko Carstens
@ 2008-02-19 15:41       ` Heiko Carstens
  2008-02-19 18:44         ` Segher Boessenkool
  0 siblings, 1 reply; 21+ messages in thread
From: Heiko Carstens @ 2008-02-19 15:41 UTC (permalink / raw)
  To: Bastian Blank, Martin Schwidefsky, linux-kernel, linux-s390

On Tue, Feb 19, 2008 at 04:38:56PM +0100, Heiko Carstens wrote:
> On Tue, Feb 19, 2008 at 04:13:55PM +0100, Bastian Blank wrote:
> > On Tue, Feb 19, 2008 at 03:40:54PM +0100, Martin Schwidefsky wrote:
> > > +	/*
> > > +	 * Only need to copy the first 512 bytes from address 0. But since
> > > +	 * the compiler emits a warning if src == NULL for memcpy use copy_page
> > > +	 * instead. Copies more than needed but this code is not performance
> > > +	 * critical.
> > > +	 */
> > > +	copy_page(lowcore, &S390_lowcore);
> > 
> > Boah, workaround alert. Why do you not fix the compiler?
> 
> We need to copy from address 0 (that's where the lowcore resides). But gcc
> insists to complain if memcpy is used with src == NULL.. Now what?

Erm sorry, misread your question. Usually it's a bug to use memcpy with
src == NULL. In this case it's ok. So I think it's perfectly ok if gcc
emits a warning.
If you know of a better way to get around this please let me know.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-19 15:41       ` Heiko Carstens
@ 2008-02-19 18:44         ` Segher Boessenkool
  2008-02-20  9:45           ` Heiko Carstens
  0 siblings, 1 reply; 21+ messages in thread
From: Segher Boessenkool @ 2008-02-19 18:44 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Bastian Blank, linux-kernel, linux-s390, Martin Schwidefsky

>>>> +	/*
>>>> +	 * Only need to copy the first 512 bytes from address 0. But since
>>>> +	 * the compiler emits a warning if src == NULL for memcpy use 
>>>> copy_page
>>>> +	 * instead. Copies more than needed but this code is not 
>>>> performance
>>>> +	 * critical.
>>>> +	 */
>>>> +	copy_page(lowcore, &S390_lowcore);
>>>
>>> Boah, workaround alert. Why do you not fix the compiler?
>>
>> We need to copy from address 0 (that's where the lowcore resides). 
>> But gcc
>> insists to complain if memcpy is used with src == NULL.. Now what?
>
> Erm sorry, misread your question. Usually it's a bug to use memcpy with
> src == NULL. In this case it's ok. So I think it's perfectly ok if gcc
> emits a warning.
> If you know of a better way to get around this please let me know.

-ffreestanding or -Wno-nonnull?


Segher


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-19 18:44         ` Segher Boessenkool
@ 2008-02-20  9:45           ` Heiko Carstens
  2008-02-20 10:09             ` Bastian Blank
  0 siblings, 1 reply; 21+ messages in thread
From: Heiko Carstens @ 2008-02-20  9:45 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Bastian Blank, linux-kernel, linux-s390, Martin Schwidefsky

>>>> Boah, workaround alert. Why do you not fix the compiler?
>>>
>>> We need to copy from address 0 (that's where the lowcore resides). But 
>>> gcc
>>> insists to complain if memcpy is used with src == NULL.. Now what?
>>
>> Erm sorry, misread your question. Usually it's a bug to use memcpy with
>> src == NULL. In this case it's ok. So I think it's perfectly ok if gcc
>> emits a warning.
>> If you know of a better way to get around this please let me know.
>
> -ffreestanding or -Wno-nonnull?

Ok, how about the patch below? Everybody happy with it?

---
 arch/s390/kernel/Makefile |    5 +++++
 arch/s390/kernel/smp.c    |    8 +-------
 2 files changed, 6 insertions(+), 7 deletions(-)

Index: linux-2.5/arch/s390/kernel/Makefile
===================================================================
--- linux-2.5.orig/arch/s390/kernel/Makefile
+++ linux-2.5/arch/s390/kernel/Makefile
@@ -4,6 +4,11 @@
 
 EXTRA_AFLAGS	:= -traditional
 
+#
+# Passing null pointers is ok for smp code, since we access the lowcore here.
+#
+CFLAGS_smp.o	:= -Wno-nonnull
+
 obj-y	:=  bitmap.o traps.o time.o process.o base.o early.o \
             setup.o sys_s390.o ptrace.o signal.o cpcmd.o ebcdic.o \
 	    semaphore.o s390_ext.o debug.o irq.o ipl.o dis.o diag.o
Index: linux-2.5/arch/s390/kernel/smp.c
===================================================================
--- linux-2.5.orig/arch/s390/kernel/smp.c
+++ linux-2.5/arch/s390/kernel/smp.c
@@ -631,13 +631,7 @@ static int __cpuinit smp_alloc_lowcore(i
 	panic_stack = __get_free_page(GFP_KERNEL);
 	if (!panic_stack || !async_stack)
 		goto out;
-	/*
-	 * Only need to copy the first 512 bytes from address 0. But since
-	 * the compiler emits a warning if src == NULL for memcpy use copy_page
-	 * instead. Copies more than needed but this code is not performance
-	 * critical.
-	 */
-	copy_page(lowcore, &S390_lowcore);
+	memcpy(lowcore, &S390_lowcore, 512);
 	memset((void *)lowcore + 512, 0, sizeof(*lowcore) - 512);
 	lowcore->async_stack = async_stack + ASYNC_SIZE;
 	lowcore->panic_stack = panic_stack + PAGE_SIZE;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-20  9:45           ` Heiko Carstens
@ 2008-02-20 10:09             ` Bastian Blank
  2008-02-20 10:24               ` Heiko Carstens
  0 siblings, 1 reply; 21+ messages in thread
From: Bastian Blank @ 2008-02-20 10:09 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Segher Boessenkool, linux-kernel, linux-s390, Martin Schwidefsky

On Wed, Feb 20, 2008 at 10:45:52AM +0100, Heiko Carstens wrote:
> -	copy_page(lowcore, &S390_lowcore);
> +	memcpy(lowcore, &S390_lowcore, 512);

Okay

>  	memset((void *)lowcore + 512, 0, sizeof(*lowcore) - 512);

Not completely okay. void pointer are not allowed in arithmetic. gcc
handles void * as char * in this case, but I think it should usualy be
avoided.

Bastian

-- 
Peace was the way.
		-- Kirk, "The City on the Edge of Forever", stardate unknown

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch 07/13] Initialize per cpu lowcores on cpu hotplug.
  2008-02-20 10:09             ` Bastian Blank
@ 2008-02-20 10:24               ` Heiko Carstens
  0 siblings, 0 replies; 21+ messages in thread
From: Heiko Carstens @ 2008-02-20 10:24 UTC (permalink / raw)
  To: Bastian Blank, Segher Boessenkool, linux-kernel, linux-s390,
	Martin Schwidefsky

On Wed, Feb 20, 2008 at 11:09:33AM +0100, Bastian Blank wrote:
> On Wed, Feb 20, 2008 at 10:45:52AM +0100, Heiko Carstens wrote:
> > -	copy_page(lowcore, &S390_lowcore);
> > +	memcpy(lowcore, &S390_lowcore, 512);
> 
> Okay
> 
> >  	memset((void *)lowcore + 512, 0, sizeof(*lowcore) - 512);
> 
> Not completely okay. void pointer are not allowed in arithmetic. gcc
> handles void * as char * in this case, but I think it should usualy be
> avoided.

There are many places all over the kernel that assume sizeof(void) == 1.
That's yet another gcc extension we use... but I'm going to change that
to a char * cast anyway.

Thanks for commenting!

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2008-02-20 10:25 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-19 14:40 [patch 00/13] s390 bux fixes for 2.6.25-rc2 Martin Schwidefsky
2008-02-19 14:40 ` [patch 01/13] cio: Remember to initialize recovery_lock Martin Schwidefsky
2008-02-19 14:40 ` [patch 02/13] cio: Do timed recovery on workqueue Martin Schwidefsky
2008-02-19 14:40 ` [patch 03/13] Let NR_CPUS default to 32/64 on s390/s390x Martin Schwidefsky
2008-02-19 14:40 ` [patch 04/13] Make sure enabled wait psw is loaded in default_idle Martin Schwidefsky
2008-02-19 14:40 ` [patch 05/13] dasd: fix locking in __dasd_device_process_final_queue Martin Schwidefsky
2008-02-19 14:40 ` [patch 06/13] find bit corner case Martin Schwidefsky
2008-02-19 14:40 ` [patch 07/13] Initialize per cpu lowcores on cpu hotplug Martin Schwidefsky
2008-02-19 15:13   ` Bastian Blank
2008-02-19 15:38     ` Heiko Carstens
2008-02-19 15:41       ` Heiko Carstens
2008-02-19 18:44         ` Segher Boessenkool
2008-02-20  9:45           ` Heiko Carstens
2008-02-20 10:09             ` Bastian Blank
2008-02-20 10:24               ` Heiko Carstens
2008-02-19 14:40 ` [patch 08/13] qdio: fix qdio_activate timeout handling Martin Schwidefsky
2008-02-19 14:40 ` [patch 09/13] etr: fix compile error on !SMP Martin Schwidefsky
2008-02-19 14:40 ` [patch 10/13] sclp: clean up send/receive naming scheme Martin Schwidefsky
2008-02-19 14:40 ` [patch 11/13] dcss: Fix Unlikely(x) != y Martin Schwidefsky
2008-02-19 14:40 ` [patch 12/13] Fix futex_atomic_cmpxchg_std inline assembly Martin Schwidefsky
2008-02-19 14:41 ` [patch 13/13] qdio: FCP/SCSI write I/O stagnates on LPAR Martin Schwidefsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).