All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* Re: Important - MM panic in 2.1.109 [PATCH + Oops]
       [not found] ` <Pine.LNX.3.95.980722160724.440A-200000@mikeg.weiden.de>
@ 1998-07-23 17:09   ` Stephen C. Tweedie
  1998-07-23 17:53     ` Linus Torvalds
  0 siblings, 1 reply; 2+ messages in thread
From: Stephen C. Tweedie @ 1998-07-23 17:09 UTC (permalink / raw
  To: Linus Torvalds, Michael L. Galbraith
  Cc: Itai Nahshon, linux kernel list, linux-mm, Stephen Tweedie

Hi,

On Wed, 22 Jul 1998 16:37:29 +0200 (MET DST), "Michael L. Galbraith"
<mikeg@weiden.de> said:

> On Fri, 17 Jul 1998, Itai Nahshon wrote:
>> 1. I sent a similar report some time ago.. the panic still happens.
>> The problem is when unmapping the last part (but not all) of
>> a segment acquired by shmget + shmat. Ref count goes down to 0
>> and then the segment is reused!!

Definitely reproducible, thanks.

> Hi Folks,

> I've attached an oops for _real_ hackers to examine. Patch follows.
> It should illuminate the problem, and at least does stop the oops.

> --- linux-2.1.111-pre1/ipc/shm.c.org	Wed Jul 22 13:07:27 1998
> +++ linux-2.1.111-pre1/ipc/shm.c	Wed Jul 22 15:56:50 1998
> @@ -626,7 +626,12 @@
>  	remove_attach(shp,shmd);  /* remove from shp->attaches */
> shp-> shm_lpid = current->pid;
> shp-> shm_dtime = CURRENT_TIME;
> -	if (--shp->shm_nattch <= 0 && shp->shm_perm.mode & SHM_DEST)
> +	/* FIXME: If vm_end = vm_start, we are doing unmap_fixup().
> +	 * This is very fragile and stupid.  It's also the best I
> +	 * could come up with without special casing unmap_fixup().
> +	 */
> +	if (--shp->shm_nattch <= 0 && shp->shm_perm.mode & SHM_DEST
> +			&& shmd->vm_end != shmd->vm_start)
>  		killseg (id);
>  }
 
This fixes the problem right enough.

However, there's an alternative fix in mm/mmap.c:

----------------------------------------------------------------
diff -u mm/mmap.c~ mm/mmap.c
--- mm/mmap.c~  Wed Jul 22 14:48:04 1998
+++ mm/mmap.c   Thu Jul 23 15:39:50 1998
@@ -421,16 +421,6 @@
                insert_vm_struct(current->mm, mpnt);
        }
 
-       /* Close the current area ... */
-       if (area->vm_ops && area->vm_ops->close) {
-               end = area->vm_end; /* save new end */
-               area->vm_end = area->vm_start;
-               area->vm_ops->close(area);
-               area->vm_end = end;
-       }
-       /* ... then reopen and reinsert. */
-       if (area->vm_ops && area->vm_ops->open)
-               area->vm_ops->open(area);
        insert_vm_struct(current->mm, area);
        return 1;
 }
----------------------------------------------------------------

This seems to be a far less messy way to deal with the underlying bug,
which arises due to the close then reopen of a vma if we unmap from one
end.  *Any* vm type which refcounts its objects will be hit by this
behaviour in unmap_fixup(), which lets the underlying mapped object's
refcount go to zero only to be brought back up to one via the same vma.

Currently the only user of vm->open and ->close is shm, and it is only
used as a reference counting mechanism.  If the reference count is not
being modified, then why should we call close/open at all, especially if
we don't even bother to supply the correct vm_start parameter to the
close call?

The oops is definitely eliminated by the second patch, although the
first one looks reasonable too.

--Stephen
--
This is a majordomo managed list.  To unsubscribe, send a message with
the body 'unsubscribe linux-mm me@address' to: majordomo@kvack.org

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

* Re: Important - MM panic in 2.1.109 [PATCH + Oops]
  1998-07-23 17:09   ` Important - MM panic in 2.1.109 [PATCH + Oops] Stephen C. Tweedie
@ 1998-07-23 17:53     ` Linus Torvalds
  0 siblings, 0 replies; 2+ messages in thread
From: Linus Torvalds @ 1998-07-23 17:53 UTC (permalink / raw
  To: Stephen C. Tweedie
  Cc: Michael L. Galbraith, Itai Nahshon, linux kernel list, linux-mm



On Thu, 23 Jul 1998, Stephen C. Tweedie wrote:
> 
> However, there's an alternative fix in mm/mmap.c:

[ patch removed ]

Applied. I always prefer to fix bug by removing code rather than adding it
;) 

		Linus

--
This is a majordomo managed list.  To unsubscribe, send a message with
the body 'unsubscribe linux-mm me@address' to: majordomo@kvack.org

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

end of thread, other threads:[~1998-07-23 17:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <35AF3BC6.E316ED09@actcom.co.il>
     [not found] ` <Pine.LNX.3.95.980722160724.440A-200000@mikeg.weiden.de>
1998-07-23 17:09   ` Important - MM panic in 2.1.109 [PATCH + Oops] Stephen C. Tweedie
1998-07-23 17:53     ` Linus Torvalds

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.