lkml.org 
[lkml]   [2000]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Subjectdeadlock in shm in pre2-4
From
Date
Hi Alan,

Your patch locks up on IPC_RMID. This should be fixed in the version I
send yesterday.

Also the meaning of deleted and IPC_DEST should be switched like done
in my patch. So ipcs will show destroyed segments as destroyed.

I append my patch against pre4.

Greetings
Christoph

--
diff -uNr 99-pre2-4/Documentation/Changes c2-3/Documentation/Changes
--- 99-pre2-4/Documentation/Changes Sat Mar 18 09:18:34 2000
+++ c2-3/Documentation/Changes Fri Mar 17 20:06:28 2000
@@ -73,8 +73,7 @@
===================

To use System V shared memory, you have to mount the shm filesystem
-somewhere and put the mountpoint into /proc/sys/kernel/shmpath.
-Default is /var/shm.
+somewhere.

<CTRL><ALT><DEL> now performs a cold reboot instead of a warm reboot
for increased hardware compatibility. If you want a warm reboot and
diff -uNr 99-pre2-4/ipc/shm.c c2-3/ipc/shm.c
--- 99-pre2-4/ipc/shm.c Sat Mar 18 09:18:36 2000
+++ c2-3/ipc/shm.c Fri Mar 17 20:04:48 2000
@@ -20,6 +20,13 @@
* can only be mounted one time.
* 3) Read and write are not implemented (should they?)
* 4) No special nodes are supported
+ *
+ * There are the following mount options:
+ * - nr_blocks (^= shmall) is the number of blocks of size PAGE_SIZE
+ * we are allowed to allocate
+ * - nr_inodes (^= shmmni) is the number of files we are allowed to
+ * allocate
+ * - mode is the mode for the root directory (default S_IRWXUGO | S_ISVTX)
*/

#include <linux/config.h>
@@ -67,7 +74,6 @@
unsigned long shm_npages; /* size of segment (pages) */
pte_t **shm_dir; /* ptr to arr of ptrs to frames */
int id;
- int destroyed; /* set if the final detach kills */
union permap {
struct shmem {
time_t atime;
@@ -75,6 +81,7 @@
time_t ctime;
pid_t cpid;
pid_t lpid;
+ int unlinked;
int nlen;
char nm[0];
} shmem;
@@ -92,6 +99,7 @@
#define shm_lprid permap.shmem.lpid
#define shm_namelen permap.shmem.nlen
#define shm_name permap.shmem.nm
+#define shm_unlinked permap.shmem.unlinked
#define zsem permap.zero.sema
#define zero_list permap.zero.list

@@ -102,6 +110,7 @@
#define shm_lockall() ipc_lockall(&shm_ids)
#define shm_unlockall() ipc_unlockall(&shm_ids)
#define shm_get(id) ((struct shmid_kernel*)ipc_get(&shm_ids,id))
+#define shm_rmid(id) ((struct shmid_kernel*)ipc_rmid(&shm_ids,id))
#define shm_checkid(s, id) \
ipc_checkid(&shm_ids,&s->shm_perm,id)
#define shm_buildid(id, seq) \
@@ -111,7 +120,7 @@
static void killseg_core(struct shmid_kernel *shp, int doacc);
static void shm_open (struct vm_area_struct *shmd);
static void shm_close (struct vm_area_struct *shmd);
-static void shm_remove_name(int id);
+static void shm_remove_name(struct shmid_kernel *shp, int id);
static struct page * shm_nopage(struct vm_area_struct *, unsigned long, int);
static int shm_swapout(struct page *, struct file *);
#ifdef CONFIG_PROC_FS
@@ -263,7 +272,7 @@
struct inode * root_inode;

if (shm_sb) {
- printk(KERN_ERR "shm fs already mounted\n");
+ printk ("shm fs already mounted\n");
return NULL;
}

@@ -271,7 +280,7 @@
shm_ctlmni = SHMMNI;
shm_mode = S_IRWXUGO | S_ISVTX;
if (shm_parse_options (data)) {
- printk(KERN_ERR "shm fs invalid option\n");
+ printk ("shm fs invalid option\n");
goto out_unlock;
}

@@ -294,7 +303,7 @@
return s;

out_no_root:
- printk(KERN_ERR "proc_read_super: get root inode failed\n");
+ printk("proc_read_super: get root inode failed\n");
iput(root_inode);
out_unlock:
return NULL;
@@ -307,16 +316,6 @@
return 0;
}

-static inline struct shmid_kernel *shm_rmid(int id)
-{
- return (struct shmid_kernel *)ipc_rmid(&shm_ids,id);
-}
-
-static __inline__ int shm_addid(struct shmid_kernel *shp)
-{
- return ipc_addid(&shm_ids, &shp->shm_perm, shm_ctlmni+1);
-}
-
static void shm_put_super(struct super_block *sb)
{
struct super_block **p = &shm_sb;
@@ -336,7 +335,7 @@
if (!(shp = shm_lock (i)))
continue;
if (shp->shm_nattch)
- printk(KERN_DEBUG "shm_nattch = %ld\n", shp->shm_nattch);
+ printk ("shm_nattch = %ld\n", shp->shm_nattch);
shp = shm_rmid(i);
shm_unlock(i);
killseg_core(shp, 1);
@@ -446,7 +445,7 @@
continue;
if (!(shp = shm_get (nr-2)))
continue;
- if (shp->shm_perm.mode & SHM_DEST)
+ if (shp->shm_unlinked)
continue;
if (filldir(dirent, shp->shm_name, shp->shm_namelen, nr, nr) < 0 )
break;;
@@ -475,7 +474,7 @@
continue;
if (!(shp = shm_lock(i)))
continue;
- if (!(shp->shm_perm.mode & SHM_DEST) &&
+ if (!(shp->shm_unlinked) &&
dent->d_name.len == shp->shm_namelen &&
strncmp(dent->d_name.name, shp->shm_name, shp->shm_namelen) == 0)
goto found;
@@ -513,6 +512,7 @@
down (&shm_ids.sem);
if (!(shp = shm_lock (inode->i_ino)))
BUG();
+ shp->shm_unlinked = 1;
shp->shm_perm.mode |= SHM_DEST;
shp->shm_perm.key = IPC_PRIVATE; /* Do not find it any more */
shm_unlock (inode->i_ino);
@@ -644,16 +644,18 @@
static inline struct shmid_kernel *newseg_alloc(int numpages, size_t namelen)
{
struct shmid_kernel *shp;
+ pte_t **dir;

shp = (struct shmid_kernel *) kmalloc (sizeof (*shp) + namelen, GFP_KERNEL);
if (!shp)
- return 0;
+ return ERR_PTR(-ENOMEM);

- shp->shm_dir = shm_alloc (numpages);
- if (!shp->shm_dir) {
+ dir = shm_alloc (numpages);
+ if (IS_ERR(dir)) {
kfree(shp);
- return 0;
+ return ERR_PTR(PTR_ERR(dir));
}
+ shp->shm_dir = dir;
shp->shm_npages = numpages;
shp->shm_nattch = 0;
shp->shm_namelen = namelen;
@@ -672,13 +674,12 @@

if (size > shm_ctlmax)
return -EINVAL;
-
if (shm_tot + numpages >= shm_ctlall)
return -ENOSPC;

- if (!(shp = newseg_alloc(numpages, namelen ? namelen : SHM_FMT_LEN + 1)))
- return -ENOMEM;
- id = shm_addid(shp);
+ if (IS_ERR(shp = newseg_alloc(numpages, namelen ? namelen : SHM_FMT_LEN + 1)))
+ return PTR_ERR(shp);
+ id = ipc_addid(&shm_ids, &shp->shm_perm, shm_ctlmni+1);
if(id == -1) {
shm_free(shp->shm_dir,numpages);
kfree(shp);
@@ -692,6 +693,7 @@
shp->shm_atim = shp->shm_dtim = 0;
shp->shm_ctim = CURRENT_TIME;
shp->id = shm_buildid(id,shp->shm_perm.seq);
+ shp->shm_unlinked = 0;
if (namelen != 0) {
shp->shm_namelen = namelen;
memcpy (shp->shm_name, name, namelen);
@@ -1015,49 +1017,29 @@
case IPC_RMID:
{
/*
- * We cannot simply remove the file. The SVID states
- * that the block remains until the last person
- * detaches from it, then is deleted. A shmat() on
- * an RMID segment is legal in older Linux and if
- * we change it apps break...
+ * A shmat() on an RMID segment is legal in older
+ * Linux and if we change it apps break...
*
- * Instead we set a destroyed flag, and then blow
- * the name away when the usage hits zero.
+ * Instead we set a destroyed flag, and then blow the
+ * name away when the usage hits zero.
*/
+
+ lock_kernel();
if ((shmid % SEQ_MULTIPLIER) == zero_id)
return -EINVAL;
- lock_kernel();
- down(&shm_ids.sem);
shp = shm_lock(shmid);
- if (shp == NULL) {
- unlock_kernel();
+ if(shp==NULL)
return -EINVAL;
- }
- err = -EIDRM;
- if (shm_checkid(shp, shmid) == 0) {
- if (shp->shm_nattch == 0) {
- int id=shp->id;
- shm_unlock(shmid);
- /* The kernel lock prevents new attaches from
- * being happening. We can't hold shm_lock here
- * else we will deadlock in shm_lookup when we
- * try to recursively grab it.
- */
- shm_remove_name(id);
- } else {
- /* Do not find me any more */
- shp->destroyed = 1;
- shp->shm_perm.key = IPC_PRIVATE; /* Do not find it any more */
- /* Unlock */
- shm_unlock(shmid);
- }
- err = 0;
- } else {
+ if(shm_checkid(shp,shmid)!=0) {
shm_unlock(shmid);
+ return -EIDRM;
}
- up(&shm_ids.sem);
+
+ shp->shm_perm.mode |= SHM_DEST;
+ shp->shm_perm.key = IPC_PRIVATE; /* Do not find it any more */
+ shm_remove_name (shp, shmid);
unlock_kernel();
- return err;
+ return 0;
}

case IPC_SET:
@@ -1155,10 +1137,8 @@
sprintf (name, SHM_FMT, shmid);
lock_kernel();
file = filp_open(name, O_RDWR, 0, dget(shm_sb->s_root));
- if (IS_ERR (file)) {
- unlock_kernel();
+ if (IS_ERR (file))
goto bad_file;
- }
*raddr = do_mmap (file, addr, file->f_dentry->d_inode->i_size,
(shmflg & SHM_RDONLY ? PROT_READ :
PROT_READ | PROT_WRITE), flags, 0);
@@ -1171,6 +1151,7 @@
return err;

bad_file:
+ unlock_kernel();
if ((err = PTR_ERR(file)) == -ENOENT)
return -EINVAL;
return err;
@@ -1183,16 +1164,21 @@
}

/*
- * Remove a name. Must be called with lock_kernel
+ * Remove a name. Must be called with lock_kernel and the shmid
+ * locked. Will unlock the shmid.
*/

-static void shm_remove_name(int id)
+static void shm_remove_name(struct shmid_kernel *shp, int id)
{
char name[SHM_FMT_LEN+1];
+
+ if(!(shp->shm_nattch == 0 && shp->shm_perm.mode & SHM_DEST)) {
+ shm_unlock(id);
+ return;
+ }
sprintf (name, SHM_FMT, id);
- if(do_unlink (name, dget(shm_sb->s_root)))
- printk(KERN_ERR "Unlink of SHM object '%s' failed.\n",
- name);
+ shm_unlock(id);
+ do_unlink (name, dget(shm_sb->s_root));
}

/*
@@ -1207,28 +1193,13 @@
struct shmid_kernel *shp;

lock_kernel();
-
/* remove from the list of attaches of the shm segment */
if(!(shp = shm_lock(id)))
BUG();
shp->shm_lprid = current->pid;
shp->shm_dtim = CURRENT_TIME;
shp->shm_nattch--;
- if(shp->shm_nattch == 0 && shp->destroyed) {
- int pid=shp->id;
- shp->destroyed = 0;
- shm_unlock(id);
-
- /* The kernel lock prevents new attaches from
- * being happening. We can't hold shm_lock here
- * else we will deadlock in shm_lookup when we
- * try to recursively grab it.
- */
- shm_remove_name(pid);
- } else {
- shm_unlock(id);
- }
-
+ shm_remove_name(shp, id);
unlock_kernel();
}

@@ -1534,8 +1505,8 @@
continue;
shp = shm_lock(i);
if(shp!=NULL) {
-#define SMALL_STRING "%10d %10d %4o %10u %5u %5u %5d %5u %5u %5u %5u %10lu %10lu %10lu %.*s\n"
-#define BIG_STRING "%10d %10d %4o %21u %5u %5u %5d %5u %5u %5u %5u %10lu %10lu %10lu %.*s\n"
+#define SMALL_STRING "%10d %10d %4o %10u %5u %5u %5d %5u %5u %5u %5u %10lu %10lu %10lu %.*s%s\n"
+#define BIG_STRING "%10d %10d %4o %21u %5u %5u %5d %5u %5u %5u %5u %10lu %10lu %10lu %.*s%s\n"
char *format;

if (sizeof(size_t) <= sizeof(int))
@@ -1558,7 +1529,8 @@
shp->shm_dtim,
shp->shm_ctim,
shp->shm_namelen,
- shp->shm_name);
+ shp->shm_name,
+ shp->shm_unlinked ? " (deleted)" : "");
shm_unlock(i);

pos += len;
@@ -1647,8 +1619,8 @@

if (!vm_enough_memory((vma->vm_end - vma->vm_start) >> PAGE_SHIFT))
return -ENOMEM;
- if (!(shp = newseg_alloc((vma->vm_end - vma->vm_start) / PAGE_SIZE, 0)))
- return -ENOMEM;
+ if (IS_ERR(shp = newseg_alloc((vma->vm_end - vma->vm_start) / PAGE_SIZE, 0)))
+ return PTR_ERR(shp);
if ((filp = file_setup(vma->vm_file, shp)) == 0) {
killseg_core(shp, 0);
return -ENOMEM;
\
 
 \ /
  Last update: 2005-03-22 13:57    [W:0.218 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site