All of lore.kernel.org
 help / color / mirror / Atom feed
* Fwd: [PATCH] fix mapping_writably_mapped()
@ 2008-12-10 21:46 Kyle McMartin
  2008-12-10 21:47 ` Matthew Wilcox
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Kyle McMartin @ 2008-12-10 21:46 UTC (permalink / raw)
  To: linux-parisc

This may explain some of the userspace issues we've been seeing.

----- Forwarded message from Hugh Dickins <hugh@veritas.com> -----

Sender: linux-arch-owner@vger.kernel.org
From: Hugh Dickins <hugh@veritas.com>
Subject: [PATCH] fix mapping_writably_mapped()
To: Linus Torvalds <torvalds@linux-foundation.org>
cc: Andrew Morton <akpm@linux-foundation.org>,
	Lee Schermerhorn <Lee.Schermerhorn@hp.com>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	stable@kernel.org
Date: Wed, 10 Dec 2008 20:48:52 +0000 (GMT)
Message-ID: <Pine.LNX.4.64.0812102043060.25282@blonde.anvils>

Lee Schermerhorn noticed yesterday that I broke the mapping_writably_mapped
test in 2.6.7!  Bad bad bug, good good find.

The i_mmap_writable count must be incremented for VM_SHARED (just as
i_writecount is for VM_DENYWRITE, but while holding the i_mmap_lock)
when dup_mmap() copies the vma for fork: it has its own more optimal
version of __vma_link_file(), and I missed this out.  So the count
was later going down to 0 (dangerous) when one end unmapped, then
wrapping negative (inefficient) when the other end unmapped.

The only impact on x86 would have been that setting a mandatory lock on
a file which has at some time been opened O_RDWR and mapped MAP_SHARED
(but not necessarily PROT_WRITE) across a fork, might fail with -EAGAIN
when it should succeed, or succeed when it should fail.

But those architectures which rely on flush_dcache_page() to flush
userspace modifications back into the page before the kernel reads it,
may in some cases have skipped the flush after such a fork - though any
repetitive test will soon wrap the count negative, in which case it will
flush_dcache_page() unnecessarily.

Fix would be a two-liner, but mapping variable added, and comment moved.

Reported-by: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

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

--- 2.6.28-rc7/kernel/fork.c	2008-11-15 23:09:30.000000000 +0000
+++ linux/kernel/fork.c	2008-12-10 12:49:13.000000000 +0000
@@ -315,17 +315,20 @@ static int dup_mmap(struct mm_struct *mm
 		file = tmp->vm_file;
 		if (file) {
 			struct inode *inode = file->f_path.dentry->d_inode;
+			struct address_space *mapping = file->f_mapping;
+
 			get_file(file);
 			if (tmp->vm_flags & VM_DENYWRITE)
 				atomic_dec(&inode->i_writecount);

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

* Re: Fwd: [PATCH] fix mapping_writably_mapped()
  2008-12-10 21:46 Fwd: [PATCH] fix mapping_writably_mapped() Kyle McMartin
@ 2008-12-10 21:47 ` Matthew Wilcox
  2008-12-11  2:26 ` [RFC] Fix warnings from various parisc and hp specific files John David Anglin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Matthew Wilcox @ 2008-12-10 21:47 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: linux-parisc

On Wed, Dec 10, 2008 at 04:46:38PM -0500, Kyle McMartin wrote:
> This may explain some of the userspace issues we've been seeing.

Yes.  Yes, it would.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

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

* [RFC] Fix warnings from various parisc and hp specific files
  2008-12-10 21:46 Fwd: [PATCH] fix mapping_writably_mapped() Kyle McMartin
  2008-12-10 21:47 ` Matthew Wilcox
@ 2008-12-11  2:26 ` John David Anglin
  2008-12-11  2:34   ` John David Anglin
  2008-12-11  8:29 ` Fwd: [PATCH] fix mapping_writably_mapped() Grant Grundler
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: John David Anglin @ 2008-12-11  2:26 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: linux-parisc

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

Attached are various fixes to correct warnings.  Mostly, they fix
warnings about ignoring return values.  There is one warning about
using a possibly uninitialized variable.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

[-- Attachment #2: ccio-dma.c.d.1 --]
[-- Type: text/plain, Size: 3362 bytes --]

diff --git a/drivers/input/keyboard/hil_kbd.c b/drivers/input/keyboard/hil_kbd.c
index 71c1971..2d1aa6b 100644
--- a/drivers/input/keyboard/hil_kbd.c
+++ b/drivers/input/keyboard/hil_kbd.c
@@ -260,7 +260,7 @@ static int hil_kbd_connect(struct serio *serio, struct serio_driver *drv)
 {
 	struct hil_kbd	*kbd;
 	uint8_t		did, *idd;
-	int		i;
+	int		err, i;
 
 	kbd = kzalloc(sizeof(*kbd), GFP_KERNEL);
 	if (!kbd)
@@ -345,7 +345,13 @@ static int hil_kbd_connect(struct serio *serio, struct serio_driver *drv)
 	}
 	clear_bit(0, kbd->dev->keybit);
 
-	input_register_device(kbd->dev);
+	err = input_register_device(kbd->dev);
+	if (err < 0) {
+		input_free_device(kbd->dev);
+		kfree(kbd);
+		return err;
+	}
+
 	printk(KERN_INFO "input: %s, ID: %d\n",
 		kbd->dev->name, did);
 
diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c
index 216a559..9a35a4c 100644
--- a/drivers/input/misc/hp_sdc_rtc.c
+++ b/drivers/input/misc/hp_sdc_rtc.c
@@ -52,6 +52,8 @@ MODULE_LICENSE("Dual BSD/GPL");
 
 #define RTC_VERSION "1.10d"
 
+#define CONFIG_EXPERIMENTAL_IOCTL 0
+
 static unsigned long epoch = 2000;
 
 static struct semaphore i8042tregs;
@@ -108,7 +110,9 @@ static int hp_sdc_rtc_do_read_bbrtc (struct rtc_time *rtctm)
 	
 	if (hp_sdc_enqueue_transaction(&t)) return -1;
 	
-	down_interruptible(&tsem);  /* Put ourselves to sleep for results. */
+	/* Put ourselves to sleep for results. */
+	if (down_interruptible(&tsem) < 0)
+		return -1;
 	
 	/* Check for nonpresence of BBRTC */
 	if (!((tseq[83] | tseq[90] | tseq[69] | tseq[76] |
@@ -175,11 +179,16 @@ static int64_t hp_sdc_rtc_read_i8042timer (uint8_t loadcmd, int numreg)
 	t.seq =			tseq;
 	t.act.semaphore =	&i8042tregs;
 
-	down_interruptible(&i8042tregs);  /* Sleep if output regs in use. */
+	/* Sleep if output regs in use. */
+	if (down_interruptible(&i8042tregs) < 0)
+		return -1;
 
 	if (hp_sdc_enqueue_transaction(&t)) return -1;
 	
-	down_interruptible(&i8042tregs);  /* Sleep until results come back. */
+	/* Sleep until results come back. */
+	if (down_interruptible(&i8042tregs) < 0)
+		return -1;
+
 	up(&i8042tregs);
 
 	return (tseq[5] | 
@@ -274,6 +283,7 @@ static inline int hp_sdc_rtc_read_ct(struct timeval *res) {
 	return 0;
 }
 
+#if CONFIG_EXPERIMENTAL_IOCTL
 
 /* Set the i8042 real-time clock */
 static int hp_sdc_rtc_set_rt (struct timeval *setto)
@@ -386,6 +396,8 @@ static int hp_sdc_rtc_set_i8042timer (struct timeval *setto, uint8_t setcmd)
 	return 0;
 }
 
+#endif /* CONFIG_EXPERIMENTAL_IOCTL */
+
 static ssize_t hp_sdc_rtc_read(struct file *file, char __user *buf,
 			       size_t count, loff_t *ppos) {
 	ssize_t retval;
@@ -517,7 +529,7 @@ static int hp_sdc_rtc_read_proc(char *page, char **start, off_t off,
 static int hp_sdc_rtc_ioctl(struct inode *inode, struct file *file, 
 			    unsigned int cmd, unsigned long arg)
 {
-#if 1
+#if !CONFIG_EXPERIMENTAL_IOCTL
 	return -EINVAL;
 #else
 	
diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index dcc1e99..a194acb 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -1028,8 +1028,10 @@ static int ccio_proc_info(struct seq_file *m, void *p)
 
 	while (ioc != NULL) {
 		unsigned int total_pages = ioc->res_size << 3;
+#ifdef CCIO_COLLECT_STATS
 		unsigned long avg = 0, min, max;
 		int j;
+#endif
 
 		len += seq_printf(m, "%s\n", ioc->name);
 		

[-- Attachment #3: hp_sdc_rtc.c.d.1 --]
[-- Type: text/plain, Size: 2138 bytes --]

diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c
index 216a559..9a35a4c 100644
--- a/drivers/input/misc/hp_sdc_rtc.c
+++ b/drivers/input/misc/hp_sdc_rtc.c
@@ -52,6 +52,8 @@ MODULE_LICENSE("Dual BSD/GPL");
 
 #define RTC_VERSION "1.10d"
 
+#define CONFIG_EXPERIMENTAL_IOCTL 0
+
 static unsigned long epoch = 2000;
 
 static struct semaphore i8042tregs;
@@ -108,7 +110,9 @@ static int hp_sdc_rtc_do_read_bbrtc (struct rtc_time *rtctm)
 	
 	if (hp_sdc_enqueue_transaction(&t)) return -1;
 	
-	down_interruptible(&tsem);  /* Put ourselves to sleep for results. */
+	/* Put ourselves to sleep for results. */
+	if (down_interruptible(&tsem) < 0)
+		return -1;
 	
 	/* Check for nonpresence of BBRTC */
 	if (!((tseq[83] | tseq[90] | tseq[69] | tseq[76] |
@@ -175,11 +179,16 @@ static int64_t hp_sdc_rtc_read_i8042timer (uint8_t loadcmd, int numreg)
 	t.seq =			tseq;
 	t.act.semaphore =	&i8042tregs;
 
-	down_interruptible(&i8042tregs);  /* Sleep if output regs in use. */
+	/* Sleep if output regs in use. */
+	if (down_interruptible(&i8042tregs) < 0)
+		return -1;
 
 	if (hp_sdc_enqueue_transaction(&t)) return -1;
 	
-	down_interruptible(&i8042tregs);  /* Sleep until results come back. */
+	/* Sleep until results come back. */
+	if (down_interruptible(&i8042tregs) < 0)
+		return -1;
+
 	up(&i8042tregs);
 
 	return (tseq[5] | 
@@ -274,6 +283,7 @@ static inline int hp_sdc_rtc_read_ct(struct timeval *res) {
 	return 0;
 }
 
+#if CONFIG_EXPERIMENTAL_IOCTL
 
 /* Set the i8042 real-time clock */
 static int hp_sdc_rtc_set_rt (struct timeval *setto)
@@ -386,6 +396,8 @@ static int hp_sdc_rtc_set_i8042timer (struct timeval *setto, uint8_t setcmd)
 	return 0;
 }
 
+#endif /* CONFIG_EXPERIMENTAL_IOCTL */
+
 static ssize_t hp_sdc_rtc_read(struct file *file, char __user *buf,
 			       size_t count, loff_t *ppos) {
 	ssize_t retval;
@@ -517,7 +529,7 @@ static int hp_sdc_rtc_read_proc(char *page, char **start, off_t off,
 static int hp_sdc_rtc_ioctl(struct inode *inode, struct file *file, 
 			    unsigned int cmd, unsigned long arg)
 {
-#if 1
+#if !CONFIG_EXPERIMENTAL_IOCTL
 	return -EINVAL;
 #else
 	

[-- Attachment #4: hil_kbd.c.d.1 --]
[-- Type: text/plain, Size: 778 bytes --]

diff --git a/drivers/input/keyboard/hil_kbd.c b/drivers/input/keyboard/hil_kbd.c
index 71c1971..2d1aa6b 100644
--- a/drivers/input/keyboard/hil_kbd.c
+++ b/drivers/input/keyboard/hil_kbd.c
@@ -260,7 +260,7 @@ static int hil_kbd_connect(struct serio *serio, struct serio_driver *drv)
 {
 	struct hil_kbd	*kbd;
 	uint8_t		did, *idd;
-	int		i;
+	int		err, i;
 
 	kbd = kzalloc(sizeof(*kbd), GFP_KERNEL);
 	if (!kbd)
@@ -345,7 +345,13 @@ static int hil_kbd_connect(struct serio *serio, struct serio_driver *drv)
 	}
 	clear_bit(0, kbd->dev->keybit);
 
-	input_register_device(kbd->dev);
+	err = input_register_device(kbd->dev);
+	if (err < 0) {
+		input_free_device(kbd->dev);
+		kfree(kbd);
+		return err;
+	}
+
 	printk(KERN_INFO "input: %s, ID: %d\n",
 		kbd->dev->name, did);
 

[-- Attachment #5: dino.c.d.1 --]
[-- Type: text/plain, Size: 595 bytes --]

diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c
index 3bc54b3..a61a37c 100644
--- a/drivers/parisc/dino.c
+++ b/drivers/parisc/dino.c
@@ -614,7 +614,9 @@ dino_fixup_bus(struct pci_bus *bus)
 			    bus->self->dev.bus_id, i,
 			    bus->self->resource[i].start,
 			    bus->self->resource[i].end);
-			pci_assign_resource(bus->self, i);
+			if (pci_assign_resource(bus->self, i) != 0) {
+				printk(KERN_WARNING "Could not assign dino bus resource\n");
+			}
 			DBG("DEBUG %s after assign %d [0x%lx,0x%lx]\n",
 			    bus->self->dev.bus_id, i,
 			    bus->self->resource[i].start,

[-- Attachment #6: 8250.c.d.1 --]
[-- Type: text/plain, Size: 586 bytes --]

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 303272a..c3120be 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1609,7 +1609,7 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
 
 static void serial_unlink_irq_chain(struct uart_8250_port *up)
 {
-	struct irq_info *i;
+	struct irq_info *i = NULL;
 	struct hlist_node *n;
 	struct hlist_head *h;
 
@@ -1624,6 +1624,7 @@ static void serial_unlink_irq_chain(struct uart_8250_port *up)
 	}
 
 	BUG_ON(n == NULL);
+	BUG_ON(i == NULL);
 	BUG_ON(i->head == NULL);
 
 	if (list_empty(i->head))

[-- Attachment #7: hp_sdc_mlc.c.d.1 --]
[-- Type: text/plain, Size: 549 bytes --]

diff --git a/drivers/input/serio/hp_sdc_mlc.c b/drivers/input/serio/hp_sdc_mlc.c
index b587e2d..15c99fb 100644
--- a/drivers/input/serio/hp_sdc_mlc.c
+++ b/drivers/input/serio/hp_sdc_mlc.c
@@ -295,8 +295,9 @@ static void hp_sdc_mlc_out(hil_mlc *mlc)
 	/* priv->tseq[3] = (mlc->ddc + 1) | HP_SDC_LPS_ACSUCC; */
 	priv->tseq[3] = 0;
 	if (mlc->opacket & HIL_CTRL_APE) {
+		int ignore;
 		priv->tseq[3] |= HP_SDC_LPC_APE_IPF;
-		down_trylock(&mlc->csem);
+		ignore = down_trylock(&mlc->csem);
 	}
  enqueue:
 	hp_sdc_enqueue_transaction(&priv->trans);

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

* Re: [RFC] Fix warnings from various parisc and hp specific files
  2008-12-11  2:26 ` [RFC] Fix warnings from various parisc and hp specific files John David Anglin
@ 2008-12-11  2:34   ` John David Anglin
  0 siblings, 0 replies; 9+ messages in thread
From: John David Anglin @ 2008-12-11  2:34 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: linux-parisc

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

On Wed, 10 Dec 2008, John David Anglin wrote:

> Attached are various fixes to correct warnings.  Mostly, they fix
> warnings about ignoring return values.  There is one warning about
> using a possibly uninitialized variable.

ccio-dma.c patch had some extra stuff.  Re-attaching.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

[-- Attachment #2: ccio-dma.c.d.1 --]
[-- Type: text/plain, Size: 446 bytes --]

diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index dcc1e99..a194acb 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -1028,8 +1028,10 @@ static int ccio_proc_info(struct seq_file *m, void *p)
 
 	while (ioc != NULL) {
 		unsigned int total_pages = ioc->res_size << 3;
+#ifdef CCIO_COLLECT_STATS
 		unsigned long avg = 0, min, max;
 		int j;
+#endif
 
 		len += seq_printf(m, "%s\n", ioc->name);
 		

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

* Re: Fwd: [PATCH] fix mapping_writably_mapped()
  2008-12-10 21:46 Fwd: [PATCH] fix mapping_writably_mapped() Kyle McMartin
  2008-12-10 21:47 ` Matthew Wilcox
  2008-12-11  2:26 ` [RFC] Fix warnings from various parisc and hp specific files John David Anglin
@ 2008-12-11  8:29 ` Grant Grundler
  2008-12-11 12:52   ` John David Anglin
  2008-12-11 21:31 ` Helge Deller
  2008-12-13 16:36 ` John David Anglin
  4 siblings, 1 reply; 9+ messages in thread
From: Grant Grundler @ 2008-12-11  8:29 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: linux-parisc

On Wed, Dec 10, 2008 at 04:46:38PM -0500, Kyle McMartin wrote:
> This may explain some of the userspace issues we've been seeing.

It seems to fix the issues I pointed out.
2.6.28-rc8 (linus' linux-2.6 git) is able to build a kernel from
scratch without segfaulting! :)
Previous 2.6.27 and 2.6.28 kernels that I tested weren't able to do that.

thanks!
grant

> 
> ----- Forwarded message from Hugh Dickins <hugh@veritas.com> -----
> 
> Sender: linux-arch-owner@vger.kernel.org
> From: Hugh Dickins <hugh@veritas.com>
> Subject: [PATCH] fix mapping_writably_mapped()
> To: Linus Torvalds <torvalds@linux-foundation.org>
> cc: Andrew Morton <akpm@linux-foundation.org>,
> 	Lee Schermerhorn <Lee.Schermerhorn@hp.com>, linux-mm@kvack.org,
> 	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
> 	stable@kernel.org
> Date: Wed, 10 Dec 2008 20:48:52 +0000 (GMT)
> Message-ID: <Pine.LNX.4.64.0812102043060.25282@blonde.anvils>
> 
> Lee Schermerhorn noticed yesterday that I broke the mapping_writably_mapped
> test in 2.6.7!  Bad bad bug, good good find.
> 
> The i_mmap_writable count must be incremented for VM_SHARED (just as
> i_writecount is for VM_DENYWRITE, but while holding the i_mmap_lock)
> when dup_mmap() copies the vma for fork: it has its own more optimal
> version of __vma_link_file(), and I missed this out.  So the count
> was later going down to 0 (dangerous) when one end unmapped, then
> wrapping negative (inefficient) when the other end unmapped.
> 
> The only impact on x86 would have been that setting a mandatory lock on
> a file which has at some time been opened O_RDWR and mapped MAP_SHARED
> (but not necessarily PROT_WRITE) across a fork, might fail with -EAGAIN
> when it should succeed, or succeed when it should fail.
> 
> But those architectures which rely on flush_dcache_page() to flush
> userspace modifications back into the page before the kernel reads it,
> may in some cases have skipped the flush after such a fork - though any
> repetitive test will soon wrap the count negative, in which case it will
> flush_dcache_page() unnecessarily.
> 
> Fix would be a two-liner, but mapping variable added, and comment moved.
> 
> Reported-by: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---
> 
>  kernel/fork.c |   15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> --- 2.6.28-rc7/kernel/fork.c	2008-11-15 23:09:30.000000000 +0000
> +++ linux/kernel/fork.c	2008-12-10 12:49:13.000000000 +0000
> @@ -315,17 +315,20 @@ static int dup_mmap(struct mm_struct *mm
>  		file = tmp->vm_file;
>  		if (file) {
>  			struct inode *inode = file->f_path.dentry->d_inode;
> +			struct address_space *mapping = file->f_mapping;
> +
>  			get_file(file);
>  			if (tmp->vm_flags & VM_DENYWRITE)
>  				atomic_dec(&inode->i_writecount);
> -
> -			/* insert tmp into the share list, just after mpnt */
> -			spin_lock(&file->f_mapping->i_mmap_lock);
> +			spin_lock(&mapping->i_mmap_lock);
> +			if (tmp->vm_flags & VM_SHARED)
> +				mapping->i_mmap_writable++;
>  			tmp->vm_truncate_count = mpnt->vm_truncate_count;
> -			flush_dcache_mmap_lock(file->f_mapping);
> +			flush_dcache_mmap_lock(mapping);
> +			/* insert tmp into the share list, just after mpnt */
>  			vma_prio_tree_add(tmp, mpnt);
> -			flush_dcache_mmap_unlock(file->f_mapping);
> -			spin_unlock(&file->f_mapping->i_mmap_lock);
> +			flush_dcache_mmap_unlock(mapping);
> +			spin_unlock(&mapping->i_mmap_lock);
>  		}
>  
>  		/*
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arch" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> ----- End forwarded message -----
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: Fwd: [PATCH] fix mapping_writably_mapped()
  2008-12-11  8:29 ` Fwd: [PATCH] fix mapping_writably_mapped() Grant Grundler
@ 2008-12-11 12:52   ` John David Anglin
  0 siblings, 0 replies; 9+ messages in thread
From: John David Anglin @ 2008-12-11 12:52 UTC (permalink / raw)
  To: Grant Grundler; +Cc: kyle, linux-parisc

> On Wed, Dec 10, 2008 at 04:46:38PM -0500, Kyle McMartin wrote:
> > This may explain some of the userspace issues we've been seeing.
> 
> It seems to fix the issues I pointed out.
> 2.6.28-rc8 (linus' linux-2.6 git) is able to build a kernel from
> scratch without segfaulting! :)
> Previous 2.6.27 and 2.6.28 kernels that I tested weren't able to do that.

That's excellent news.  I applied it to my 2.6.22.19 tree.  My first
gcc build had an unexpected segfault, but the second try was ok.

I'll try 2.6.28-rc8 soon.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

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

* Re: Fwd: [PATCH] fix mapping_writably_mapped()
  2008-12-10 21:46 Fwd: [PATCH] fix mapping_writably_mapped() Kyle McMartin
                   ` (2 preceding siblings ...)
  2008-12-11  8:29 ` Fwd: [PATCH] fix mapping_writably_mapped() Grant Grundler
@ 2008-12-11 21:31 ` Helge Deller
  2008-12-11 22:03   ` James Bottomley
  2008-12-13 16:36 ` John David Anglin
  4 siblings, 1 reply; 9+ messages in thread
From: Helge Deller @ 2008-12-11 21:31 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: linux-parisc

Kyle McMartin wrote:
> This may explain some of the userspace issues we've been seeing.

It sadly does not fix the kernel panic I reported in
http://marc.info/?l=linux-parisc&m=122877889304862&w=2
("2.6.28: BUG: soft lockup - CPU#0 stuck for 61s!")

Helge

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

* Re: Fwd: [PATCH] fix mapping_writably_mapped()
  2008-12-11 21:31 ` Helge Deller
@ 2008-12-11 22:03   ` James Bottomley
  0 siblings, 0 replies; 9+ messages in thread
From: James Bottomley @ 2008-12-11 22:03 UTC (permalink / raw)
  To: Helge Deller; +Cc: Kyle McMartin, linux-parisc

On Thu, 2008-12-11 at 22:31 +0100, Helge Deller wrote:
> Kyle McMartin wrote:
> > This may explain some of the userspace issues we've been seeing.
> 
> It sadly does not fix the kernel panic I reported in
> http://marc.info/?l=linux-parisc&m=122877889304862&w=2
> ("2.6.28: BUG: soft lockup - CPU#0 stuck for 61s!")

It's not really likely to ... what you have is too much work going on in
the locking functions.  Either because of contention or debugging.

Can you reproduce it without spinlock debugging?  If yes, then we may
have some nasty accidental contention in the flushing spinlocks.

James



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

* Re: Fwd: [PATCH] fix mapping_writably_mapped()
  2008-12-10 21:46 Fwd: [PATCH] fix mapping_writably_mapped() Kyle McMartin
                   ` (3 preceding siblings ...)
  2008-12-11 21:31 ` Helge Deller
@ 2008-12-13 16:36 ` John David Anglin
  4 siblings, 0 replies; 9+ messages in thread
From: John David Anglin @ 2008-12-13 16:36 UTC (permalink / raw)
  To: Kyle McMartin; +Cc: linux-parisc

> This may explain some of the userspace issues we've been seeing.

There are still problems.  I built 2.6.28-rc8 using James' config for ion.
In my first attempt at building GCC, sh segfaulted in copy_word_list.
The kernel and sh then got into a continuous loop of SIGSEGVs filling
the file system with log messages.

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND           
27608 dave      20   0  3484 1304  512 R  100  0.0 519:33.44 sh                 

dave@mx3210:~$ strace -p 27608
Process 27608 attached - interrupt to quit
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
rt_sigreturn()                          = 1746504
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
rt_sigreturn()                          = 1746504
...

dave@mx3210:~$ gdb /bin/sh 27608
GNU gdb (GDB) 6.8.50.20080915-cvs
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "hppa-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
(no debugging symbols found)
Attaching to program: /bin/sh, process 27608

Program received signal SIGSEGV, Segmentation fault.
Reading symbols from /lib/libncurses.so.5...(no debugging symbols found)...done.
Loaded symbols for /lib/libncurses.so.5
Reading symbols from /lib/libdl.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib/libdl.so.2
Reading symbols from /lib/libc.so.6...
(no debugging symbols found)...done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib/ld.so.1...(no debugging symbols found)...done.
Loaded symbols for /lib/ld.so.1

0x0004110c in copy_word_list ()
(gdb) bt
#0  0x0004110c in copy_word_list ()
#1  0x00052a5c in ?? ()
#2  0x0003a06c in ?? ()
#3  0x00037398 in execute_command_internal ()
#4  0x00036d24 in ?? ()
#5  0x00038284 in execute_command_internal ()
#6  0x00073994 in parse_and_execute ()
#7  0x0004d810 in command_substitute ()
#8  0x00051db0 in ?? ()
#9  0x000532c8 in ?? ()
#10 0x00056f18 in expand_string_assignment ()
#11 0x00050f14 in ?? ()
#12 0x000512cc in ?? ()
#13 0x00053220 in ?? ()
#14 0x0003a06c in ?? ()
#15 0x00037398 in execute_command_internal ()
#16 0x00038608 in execute_command ()
#17 0x000382f0 in execute_command_internal ()
#18 0x00038608 in execute_command ()
#19 0x000382f0 in execute_command_internal ()
#20 0x00038608 in execute_command ()
#21 0x000382f0 in execute_command_internal ()
#22 0x00038608 in execute_command ()
---Type <return> to continue, or q <return> to quit---
#23 0x000382f0 in execute_command_internal ()
#24 0x00038608 in execute_command ()
#25 0x00037f90 in execute_command_internal ()
#26 0x00038608 in execute_command ()
#27 0x000279a8 in reader_loop ()
#28 0x000274fc in main ()
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x0004110c in copy_word_list ()
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x0004110c in copy_word_list ()
(gdb) p/x $pc
$1 = 0x4110c
(gdb) disass 0x410fc 0x4111c
Dump of assembler code from 0x410fc to 0x4111c:
0x000410fc <copy_word_list+20>:	movb,= r26,r3,0x41158 <copy_word_list+112>
0x00041100 <copy_word_list+24>:	ldi 0,r5
0x00041104 <copy_word_list+28>:	ldw 4(r3),r4
0x00041108 <copy_word_list+32>:	b,l 0x32be4 <make_bare_word>,rp
0x0004110c <copy_word_list+36>:	ldw 0(r4),r26
0x00041110 <copy_word_list+40>:	ldw 4(r4),r19
0x00041114 <copy_word_list+44>:	copy r5,r25
0x00041118 <copy_word_list+48>:	stw r19,4(ret0)
End of assembler dump.
(gdb) p/x $r4
$2 = 0x0
(gdb) disass
Dump of assembler code for function copy_word_list:
0x000410e8 <copy_word_list+0>:	stw rp,-14(sp)
0x000410ec <copy_word_list+4>:	ldi 0,r19
0x000410f0 <copy_word_list+8>:	stw,ma r5,40(sp)
0x000410f4 <copy_word_list+12>:	stw r4,-3c(sp)
0x000410f8 <copy_word_list+16>:	stw r3,-38(sp)
0x000410fc <copy_word_list+20>:	movb,= r26,r3,0x41158 <copy_word_list+112>
0x00041100 <copy_word_list+24>:	ldi 0,r5
0x00041104 <copy_word_list+28>:	ldw 4(r3),r4
0x00041108 <copy_word_list+32>:	b,l 0x32be4 <make_bare_word>,rp
0x0004110c <copy_word_list+36>:	ldw 0(r4),r26
0x00041110 <copy_word_list+40>:	ldw 4(r4),r19
0x00041114 <copy_word_list+44>:	copy r5,r25
0x00041118 <copy_word_list+48>:	stw r19,4(ret0)
0x0004111c <copy_word_list+52>:	b,l 0x32278 <make_word_list>,rp
0x00041120 <copy_word_list+56>:	copy ret0,r26
0x00041124 <copy_word_list+60>:	ldw 0(r3),r3
0x00041128 <copy_word_list+64>:	cmpib,<> 0,r3,0x41104 <copy_word_list+28>
0x0004112c <copy_word_list+68>:	copy ret0,r5
0x00041130 <copy_word_list+72>:	cmpib,= 0,ret0,0x41158 <copy_word_list+112>
0x00041134 <copy_word_list+76>:	copy ret0,r19
0x00041138 <copy_word_list+80>:	ldw 0(ret0),ret0
0x0004113c <copy_word_list+84>:	cmpib,= 0,ret0,0x41158 <copy_word_list+112>
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb) p/x $r3
$3 = 0xd0e3c
(gdb) x/4x 0xd0e3c
0xd0e3c <error_trace_mode>:	0x00000000	0x00000000	0x000e1928	0x0019b2c8

Dec 13 01:32:21 mx3210 kernel: 
Dec 13 01:32:21 mx3210 kernel: do_page_fault() pid=27608 command='sh' type=15 address=0x00000000
Dec 13 01:32:21 mx3210 kernel: 
Dec 13 01:32:21 mx3210 kernel:      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
Dec 13 01:32:21 mx3210 kernel: PSW: 00000000000011100000000000001111 Tainted: G        W 
Dec 13 01:32:21 mx3210 kernel: r00-03  00000000000e000f 00000000000d06a0 0000000000041113 00000000000d0e3c
Dec 13 01:32:21 mx3210 kernel: r04-07  0000000000000000 00000000001aa648 00000000000200b0 000000000019a508
Dec 13 01:32:21 mx3210 kernel: r08-11  00000000000d06a0 0000000000000001 00000000000c9ea0 00000000000d3b60
Dec 13 01:32:21 mx3210 kernel: r12-15  0000000000000000 0000000000000000 00000000000c9ea0 000000000000001f
Dec 13 01:32:21 mx3210 kernel: r16-19  00000000000d06a0 0000000000000001 0000000000000003 00000000001aa648
Dec 13 01:32:21 mx3210 kernel: r20-23  000000000019a1d2 0000000040762614 000000000019a1d2 0000000000000001
Dec 13 01:32:21 mx3210 kernel: r24-27  000000000000000b 0000000000000000 000000000019a588 00000000000c9ea0
Dec 13 01:32:21 mx3210 kernel: r28-31  00000000001aa648 0000000000000040 00000000faff4800 0000000000000003
Dec 13 01:32:21 mx3210 kernel: sr00-03  0000000001b4e800 0000000000000000 0000000000000000 0000000001b4e800
Dec 13 01:32:21 mx3210 kernel: sr04-07  0000000001b4e800 0000000001b4e800 0000000001b4e800 0000000001b4e800
Dec 13 01:32:21 mx3210 kernel: 
Dec 13 01:32:21 mx3210 kernel:       VZOUICununcqcqcqcqcqcrmunTDVZOUI
Dec 13 01:32:21 mx3210 kernel: FPSR: 00000000000000000000000000000000
Dec 13 01:32:21 mx3210 kernel: FPER1: 00000000
Dec 13 01:32:21 mx3210 kernel: fr00-03  0000000000000000 0000000000000000 0000000000000000 0000000000000000
Dec 13 01:32:21 mx3210 kernel: fr04-07  fffffffffffff000 0000000000000000 ffffffffffffff9c bff0000000000000
Dec 13 01:32:21 mx3210 kernel: fr08-11  0000000000000000 00000000405b4760 000000Dec 12 23:11:11 mx3210 kernel: sd 1:0:2:0: [sdb] Mode Sense: ab 00 10 08
Dec 12 23:11:11 mx3210 kernel: sd 1:0:2:0: [sdb] Mode Sense: ab 00 10 08Dec 13 01:32:21 mx3210 kernel: 
Dec 13 01:32:21 mx3210 kernel: do_page_fault() pid=27608 command='sh' type=15 address=0x00000000
Dec 13 01:32:21 mx3210 kernel: Dec 13 01:32:21 mx3210 kernel:      YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
Dec 13 01:32:21 mx3210 kernel: PSW: 00000000000011100000000000001111 Tainted: G        W 
Dec 13 01:32:21 mx3210 kernel: r00-03  00000000000e000f 00000000000d06a0 0000000000041113 00000000000d0e3c
Dec 13 01:32:21 mx3210 kernel: r04-07  0000000000000000 00000000001aa648 0000000
0000200b0 000000000019a508Dec 13 01:32:21 mx3210 kernel: r08-11  00000000000d06a0 0000000000000001 0000000
0000c9ea0 00000000000d3b60Dec 13 01:32:21 mx3210 kernel: r12-15  0000000000000000 0000000000000000 0000000
0000c9ea0 000000000000001fDec 13 01:32:21 mx3210 kernel: r16-19  00000000000d06a0 0000000000000001 0000000
000000003 00000000001aa648
Dec 13 01:32:21 mx3210 kernel: r20-23  000000000019a1d2 0000000040762614 0000000
00019a1d2 0000000000000001
Dec 13 01:32:21 mx3210 kernel: r24-27  000000000000000b 0000000000000000 0000000
00019a588 00000000000c9ea0
Dec 13 01:32:21 mx3210 kernel: r28-31  00000000001aa648 0000000000000040 00000000faff4800 0000000000000003
Dec 13 01:32:21 mx3210 kernel: sr00-03  0000000001b4e800 0000000000000000 0000000000000000 0000000001b4e800
Dec 13 01:32:21 mx3210 kernel: sr04-07  0000000001b4e800 0000000001b4e800 0000000001b4e800 0000000001b4e800
Dec 13 01:32:21 mx3210 kernel: 
Dec 13 01:32:21 mx3210 kernel:       VZOUICununcqcqcqcqcqcrmunTDVZOUI
Dec 13 01:32:21 mx3210 kernel: FPSR: 00000000000000000000000000000000
Dec 13 01:32:21 mx3210 kernel: FPER1: 00000000
Dec 13 01:32:21 mx3210 kernel: fr00-03  0000000000000000 0000000000000000 0000000000000000 0000000000000000
Dec 13 01:32:21 mx3210 kernel: fr04-07  fffffffffffff000 0000000000000000 ffffffffffffff9c bff0000000000000
Dec 13 01:32:21 mx3210 kernel: fr08-11  0000000000000000 00000000405b4760 0000000000000802 00000000405f2000
Dec 13 01:32:21 mx3210 kernel: fr12-15  00000000401c61b8 00000000405b4760 00000000401c4114 000000007f410d00
Dec 13 01:32:21 mx3210 kernel: fr16-19  000000007f4acb90 000000007f410d00 000000007f54c000 000000007f54c000
Dec 13 01:32:21 mx3210 kernel: fr20-23  00000000405b4760 00000000404a91d8 0000000000000040 0000000000000000
Dec 13 01:32:21 mx3210 kernel: fr24-27  0000000000000000 00000000405b4760 00000000405b4760 0000000000000000
Dec 13 01:32:21 mx3210 kernel: fr28-31  0000000000000803 0000000000000000 00000000405b4760 0000000000000803
Dec 13 01:32:21 mx3210 kernel: 
Dec 13 01:32:21 mx3210 kernel: IASQ: 0000000001b4e800 0000000001b4e800 IAOQ: 000000000004110f 0000000000032be7
Dec 13 01:32:21 mx3210 kernel:  IIR: 0c80109a    ISR: 0000000001b4e800  IOR: 0000000000000000
Dec 13 01:32:21 mx3210 kernel:  CPU:        1   CR30: 0000000044494000 CR31: ffffffffffffffff
Dec 13 01:32:21 mx3210 kernel:  ORIG_R28: 0000000000000000
Dec 13 01:32:21 mx3210 kernel:  IAOQ[0]: 000000000004110f
Dec 13 01:32:21 mx3210 kernel:  IAOQ[1]: 0000000000032be7
Dec 13 01:32:21 mx3210 kernel:  RP(r2): 0000000000041113
Dec 13 01:32:21 mx3210 kernel: 
Dec 13 01:32:21 mx3210 kernel: do_page_fault() pid=27608 command='sh' type=15 address=0x00000000
...

This might be a bash bug.

WORD_DESC *
copy_word (w)
     WORD_DESC *w;
{
  WORD_DESC *new_word;

  new_word = make_bare_word (w->word);
  new_word->flags = w->flags;
  return (new_word);
}

/* Copy the chain of words in LIST.  Return a pointer to
   the new chain. */
WORD_LIST *
copy_word_list (list)
     WORD_LIST *list;
{
  WORD_LIST *new_list;

  for (new_list = (WORD_LIST *)NULL; list; list = list->next)
    new_list = make_word_list (copy_word (list->word), new_list);

  return (REVERSE_LIST (new_list, WORD_LIST *));
}

There's no check that list->word isn't NULL.  However, allocation failures
are supposed to be caught by xmalloc.  So, it's not obvious how a NULL
pointer actually arises.

There's also a puzzle as to why bash tries to continue after the segv,
possibly at the faulting insn.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

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

end of thread, other threads:[~2008-12-13 16:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-10 21:46 Fwd: [PATCH] fix mapping_writably_mapped() Kyle McMartin
2008-12-10 21:47 ` Matthew Wilcox
2008-12-11  2:26 ` [RFC] Fix warnings from various parisc and hp specific files John David Anglin
2008-12-11  2:34   ` John David Anglin
2008-12-11  8:29 ` Fwd: [PATCH] fix mapping_writably_mapped() Grant Grundler
2008-12-11 12:52   ` John David Anglin
2008-12-11 21:31 ` Helge Deller
2008-12-11 22:03   ` James Bottomley
2008-12-13 16:36 ` John David Anglin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.