Skip to contentSkip to conversation

please pull PowerPC trees

PATCH In review Hollis Blanchard 2 participants 4 messages Jul 6, 2007 – Jul 9, 2007 Xen hypervisor interface

lore
Share & tools
lore
#1 lore raw

Changes inside powerpc files are fine. Obviously powerpc-specific changes inside common files are usually fine. Changes to locking protocols in common files (e.g., changed usage of mmap_sem in privcmd.c) is *not* fine. Please post patches for that kind of thing.

-- Keir

Quoted 11 lines · Hollis Blanchard

Hi Keir, thanks for merging the Linux patches we've been discussing.

Please pull PowerPC Xen Linux support from
http://xenbits.xensource.com/ext/ppc/linux-2.6.18-xen.hg

Then, please pull from
http://xenbits.xensource.com/ext/ppc/xen-unstable.hg
Among other things, it includes changes necessary to run the new Linux 2.6.18 kernel.

Thanks!

author
#2 lore raw

PATCH 1/1 Take a writer lock for mmap_sem.

Quoted 4 lines · Keir Fraser

Changes inside powerpc files are fine. Obviously powerpc-specific changes inside common files are usually fine. Changes to locking protocols in common files (e.g., changed usage of mmap_sem in privcmd.c) is *not* fine. Please post patches for that kind of thing.

My mistake, I meant to send this separately.

In 2.6.17, we did our own locking inside direct_remap_pfn_range(). In the current Linux tree though, the locking is done in privcmd_ioctl().

However, since direct_remap_pfn_range() is modifying the mm_struct, shouldn't that be a a write lock rather than a read lock?

[XEN][LINUX] Take a writer lock for mmap_sem. direct_remap_pfn_range() will be modifying the mm.

Signed-off-by Christian Ehrhardt <[email protected]>
Signed-off-by Hollis Blanchard <[email protected]>
drivers/xen/privcmd/privcmd.c +6 -6
diff -r e5f633c33025 drivers/xen/privcmd/privcmd.c
@@ -111,7 +112,7 @@ static int privcmd_ioctl(struct inode *i
 		if (copy_from_user(&msg, p, sizeof(msg)))
 			return -EFAULT;
 
-		down_read(&mm->mmap_sem);
+		down_write(&mm->mmap_sem);
 
 		vma = find_vma(mm, msg.va);
 		rc = -EINVAL;

@@ -153,7 +154,7 @@ static int privcmd_ioctl(struct inode *i
 		rc = 0;
 
 	mmap_out:
-		up_read(&mm->mmap_sem);
+		up_write(&mm->mmap_sem);
 		ret = rc;
 	}
 	break;

@@ -176,14 +177,14 @@ static int privcmd_ioctl(struct inode *i
 		if ((m.num <= 0) || (nr_pages > (LONG_MAX >> PAGE_SHIFT)))
 			return -EINVAL;
 
-		down_read(&mm->mmap_sem);
+		down_write(&mm->mmap_sem);
 
 		vma = find_vma(mm, m.addr);
 		if (!vma ||
 		    (m.addr != vma->vm_start) ||
 		    ((m.addr + (nr_pages << PAGE_SHIFT)) != vma->vm_end) ||
 		    !privcmd_enforce_singleshot_mapping(vma)) {
-			up_read(&mm->mmap_sem);
+			up_write(&mm->mmap_sem);
 			return -EINVAL;
 		}
 

@@ -191,7 +192,7 @@ static int privcmd_ioctl(struct inode *i
 		addr = m.addr;
 		for (i = 0; i < nr_pages; i++, addr += PAGE_SIZE, p++) {
 			if (get_user(mfn, p)) {
-				up_read(&mm->mmap_sem);
+				up_write(&mm->mmap_sem);
 				return -EFAULT;
 			}
 

@@ -202,7 +203,7 @@ static int privcmd_ioctl(struct inode *i
 				put_user(0xF0000000 | mfn, p);
 		}
 
-		up_read(&mm->mmap_sem);
+		up_write(&mm->mmap_sem);
 		ret = 0;
 	}
 	break;
Signature · 2 lines
Hollis Blanchard
IBM Linux Technology Center
#3 lore raw

Re: [PATCH] Take a writer lock for mmap_sem.

Quoted 8 lines · Hollis Blanchard

My mistake, I meant to send this separately.

In 2.6.17, we did our own locking inside direct_remap_pfn_range(). In the current Linux tree though, the locking is done in privcmd_ioctl().

However, since direct_remap_pfn_range() is modifying the mm_struct, shouldn't that be a a write lock rather than a read lock?

Ah yes, agreed. All other callers seem to get this implicitly because they are ->mmap handlers.

-- Keir