Re: [Dri-devel] 2.6 kernel change in nopage

Re: [Dri-devel] 2.6 kernel change in nopage

[Michel Dänzer]

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

On Wed, 2003-12-31 at 19:21, Jon Smirl wrote:
> The headers for nopageXX calls just changed.
> 
> struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int
> unused);
> changed to:
> struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int
> *type);
> 
> The DRM drivers need to be adjusted. This probably impacts the BSD builds.

No, this is Linux specific.

How does this patch look?


-- 
Earthling Michel Dänzer      |     Debian (powerpc), X and DRI developer
Software libre enthusiast    |   http://svcs.affero.net/rm.php?r=daenzer

[-- Attachment #2: drm-nopage.diff --]
[-- Type: text/x-patch, Size: 6417 bytes --]

Index: drmP.h
===================================================================
RCS file: /cvs/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h,v
retrieving revision 1.85
diff -p -u -r1.85 drmP.h
--- drmP.h	5 Nov 2003 08:13:52 -0000	1.85
+++ drmP.h	1 Jan 2004 12:02:05 -0000
@@ -806,18 +819,10 @@ extern int	     DRM(flush)(struct file *
 extern int	     DRM(fasync)(int fd, struct file *filp, int on);
 
 				/* Mapping support (drm_vm.h) */
-extern struct page *DRM(vm_nopage)(struct vm_area_struct *vma,
-				   unsigned long address,
-				   int write_access);
-extern struct page *DRM(vm_shm_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_dma_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_sg_nopage)(struct vm_area_struct *vma,
-				      unsigned long address,
-				      int write_access);
+extern struct page   *DRM(vm_nopage)( DRM_NOPAGE_ARGS );
+extern struct page   *DRM(vm_shm_nopage)( DRM_NOPAGE_ARGS );
+extern struct page   *DRM(vm_dma_nopage)( DRM_NOPAGE_ARGS );
+extern struct page   *DRM(vm_sg_nopage)( DRM_NOPAGE_ARGS );
 extern void	     DRM(vm_open)(struct vm_area_struct *vma);
 extern void	     DRM(vm_close)(struct vm_area_struct *vma);
 extern void	     DRM(vm_shm_close)(struct vm_area_struct *vma);
Index: drm_os_linux.h
===================================================================
RCS file: /cvs/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_os_linux.h,v
retrieving revision 1.19
diff -p -u -r1.19 drm_os_linux.h
--- drm_os_linux.h	4 Nov 2003 00:46:05 -0000	1.19
+++ drm_os_linux.h	1 Jan 2004 12:02:05 -0000
@@ -52,6 +52,13 @@ typedef void irqreturn_t;
 #define DRM_AGP_KERN		struct agp_kern_info
 #endif
 
+/** Page fault handler arguments */
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+#define DRM_NOPAGE_ARGS		struct vm_area_struct *vma, unsigned long address, int *type
+#else
+#define DRM_NOPAGE_ARGS		struct vm_area_struct *vma, unsigned long address, int unused
+#endif
+
 /** Task queue handler arguments */
 #define DRM_TASKQUEUE_ARGS	void *arg
 
Index: drm_vm.h
===================================================================
RCS file: /cvs/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_vm.h,v
retrieving revision 1.26
diff -p -u -r1.26 drm_vm.h
--- drm_vm.h	12 Sep 2003 20:00:59 -0000	1.26
+++ drm_vm.h	1 Jan 2004 12:02:05 -0000
@@ -69,15 +69,14 @@ struct vm_operations_struct   DRM(vm_sg_
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Find the right map and if it's AGP memory find the real physical page to
  * map, get the page, increment the use count and return it.
  */
-struct page *DRM(vm_nopage)(struct vm_area_struct *vma,
-			    unsigned long address,
-			    int write_access)
+struct page *DRM(vm_nopage)( DRM_NOPAGE_ARGS )
 {
 #if __REALLY_HAVE_AGP
 	drm_file_t *priv  = vma->vm_file->private_data;
@@ -134,6 +133,10 @@ struct page *DRM(vm_nopage)(struct vm_ar
 			  baddr, __va(agpmem->memory->memory[offset]), offset,
 			  atomic_read(&page->count));
 
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+		if (type)
+			*type = VM_FAULT_MINOR;
+#endif
 		return page;
         }
 vm_nopage_error:
@@ -147,15 +150,14 @@ vm_nopage_error:
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Get the the mapping, find the real physical page to map, get the page, and
  * return it.
  */
-struct page *DRM(vm_shm_nopage)(struct vm_area_struct *vma,
-				unsigned long address,
-				int write_access)
+struct page *DRM(vm_shm_nopage)( DRM_NOPAGE_ARGS )
 {
 	drm_map_t	 *map	 = (drm_map_t *)vma->vm_private_data;
 	unsigned long	 offset;
@@ -171,6 +173,10 @@ struct page *DRM(vm_shm_nopage)(struct v
 	if (!page)
 		return NOPAGE_OOM;
 	get_page(page);
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+	if (type)
+		*type = VM_FAULT_MINOR;
+#endif
 
 	DRM_DEBUG("shm_nopage 0x%lx\n", address);
 	return page;
@@ -262,14 +268,13 @@ void DRM(vm_shm_close)(struct vm_area_st
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Determine the page number from the page offset and get it from drm_device_dma::pagelist.
  */
-struct page *DRM(vm_dma_nopage)(struct vm_area_struct *vma,
-				unsigned long address,
-				int write_access)
+struct page *DRM(vm_dma_nopage)( DRM_NOPAGE_ARGS )
 {
 	drm_file_t	 *priv	 = vma->vm_file->private_data;
 	drm_device_t	 *dev	 = priv->dev;
@@ -288,6 +293,10 @@ struct page *DRM(vm_dma_nopage)(struct v
 			     (offset & (~PAGE_MASK))));
 
 	get_page(page);
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+	if (type)
+		*type = VM_FAULT_MINOR;
+#endif
 
 	DRM_DEBUG("dma_nopage 0x%lx (page %lu)\n", address, page_nr);
 	return page;
@@ -298,14 +307,13 @@ struct page *DRM(vm_dma_nopage)(struct v
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Determine the map offset from the page offset and get it from drm_sg_mem::pagelist.
  */
-struct page *DRM(vm_sg_nopage)(struct vm_area_struct *vma,
-			       unsigned long address,
-			       int write_access)
+struct page *DRM(vm_sg_nopage)( DRM_NOPAGE_ARGS )
 {
 	drm_map_t        *map    = (drm_map_t *)vma->vm_private_data;
 	drm_file_t *priv = vma->vm_file->private_data;
@@ -326,6 +334,10 @@ struct page *DRM(vm_sg_nopage)(struct vm
 	page_offset = (offset >> PAGE_SHIFT) + (map_offset >> PAGE_SHIFT);
 	page = entry->pagelist[page_offset];
 	get_page(page);
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+	if (type)
+		*type = VM_FAULT_MINOR;
+#endif
 
 	return page;
 }

Re: [Dri-devel] 2.6 kernel change in nopage

[Arjan van de Ven]

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

On Thu, 2004-01-01 at 13:03, Michel Dänzer wrote:

> How does this patch look?

ugly.

I find using #defines for function arguments ugly beyond belief and
makes it really hard to look through code. I 10x rather have an ifdef in
the function prototype (which then for the mainstream kernel drm can be
removed for non-matching versions) than such obfuscation.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Dri-devel] 2.6 kernel change in nopage

[Michel Dänzer]

On Thu, 2004-01-01 at 13:10, Arjan van de Ven wrote:
> On Thu, 2004-01-01 at 13:03, Michel Dänzer wrote:
> 
> > How does this patch look?
> 
> ugly.
> 
> I find using #defines for function arguments ugly beyond belief and
> makes it really hard to look through code. I 10x rather have an ifdef in
> the function prototype (which then for the mainstream kernel drm can be
> removed for non-matching versions) than such obfuscation.

That doesn't strike me as particularly beautiful either... is it really
easier for merges, considering that the ugly way is kinda needed for
functions which take different arguments on BSD anyway?


-- 
Earthling Michel Dänzer      |     Debian (powerpc), X and DRI developer
Software libre enthusiast    |   http://svcs.affero.net/rm.php?r=daenzer

Re: [Dri-devel] 2.6 kernel change in nopage

[Arjan van de Ven]

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

On Thu, Jan 01, 2004 at 01:23:40PM +0100, Michel Dänzer wrote:
> > > How does this patch look?
> > 
> > ugly.
> > 
> > I find using #defines for function arguments ugly beyond belief and
> > makes it really hard to look through code. I 10x rather have an ifdef in
> > the function prototype (which then for the mainstream kernel drm can be
> > removed for non-matching versions) than such obfuscation.
> 
> That doesn't strike me as particularly beautiful either... 

well the advantage is that the ifdefs can just go away in kernel trees of
specific versions... (eg unifdef it)

> is it really easier for merges, considering that the ugly way is kinda
> needed for functions which take different arguments on BSD anyway?

I disagree there. The "BSD takes different arguments" thing *should* be
fixed imo by making the common core of the function an inline function, and have
one or two (depends if the common core happens to have its arguments in common
with one of the oses) OS specific wrappers with the right prototype. This
way the difference in error return sign can also be solved in the wrapper
instead of with a nasty macro...

The compiler generates the same code, but it's a lot easier to read/review.

Greetings,
    Arjan van de Ven

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [Dri-devel] 2.6 kernel change in nopage

[William Lee Irwin III]

On Thu, Jan 01, 2004 at 01:03:38PM +0100, Michel D?nzer wrote:
> No, this is Linux specific.
> How does this patch look?

Okay, you did something weird with nopage args, but I thought I did
the equivalent of this in the original patch?


-- wli

Re: [Dri-devel] 2.6 kernel change in nopage

[Michel Dänzer]

On Thu, 2004-01-01 at 14:33, William Lee Irwin III wrote:
> On Thu, Jan 01, 2004 at 01:03:38PM +0100, Michel D?nzer wrote:
> > No, this is Linux specific.
> > How does this patch look?
> 
> Okay, you did something weird with nopage args, but I thought I did
> the equivalent of this in the original patch?

This is about the canonical DRM code in the DRI tree.


-- 
Earthling Michel Dänzer      |     Debian (powerpc), X and DRI developer
Software libre enthusiast    |   http://svcs.affero.net/rm.php?r=daenzer

Re: [Dri-devel] 2.6 kernel change in nopage

[William Lee Irwin III]

On Thu, 2004-01-01 at 14:33, William Lee Irwin III wrote:
>> Okay, you did something weird with nopage args, but I thought I did
>> the equivalent of this in the original patch?

On Thu, Jan 01, 2004 at 02:50:30PM +0100, Michel D?nzer wrote:
> This is about the canonical DRM code in the DRI tree.

I'm sorry, I'm not going to drive myself insane trying to deal with
yet another overweight and insanely ugly BSD emulation layer. I'll
stick with canonical kernel.org sources, thank you very much.


-- wli

Re: [Dri-devel] 2.6 kernel change in nopage

[Michel Dänzer]

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

On Thu, 2004-01-01 at 13:28, Arjan van de Ven wrote:
> On Thu, Jan 01, 2004 at 01:23:40PM +0100, Michel Dänzer wrote:
> > > 
> > > I find using #defines for function arguments ugly beyond belief and
> > > makes it really hard to look through code. I 10x rather have an ifdef in
> > > the function prototype (which then for the mainstream kernel drm can be
> > > removed for non-matching versions) than such obfuscation.
> > 
> > That doesn't strike me as particularly beautiful either... 
> 
> well the advantage is that the ifdefs can just go away in kernel trees of
> specific versions... (eg unifdef it)

Does this look better? Maybe a macro (or a typedef?) for the type of the
last argument would still be a good idea? Or is there yet a better way?


> > is it really easier for merges, considering that the ugly way is kinda
> > needed for functions which take different arguments on BSD anyway?
> 
> I disagree there. The "BSD takes different arguments" thing *should* be
> fixed imo by making the common core of the function an inline function, and have
> one or two (depends if the common core happens to have its arguments in common
> with one of the oses) OS specific wrappers with the right prototype. This
> way the difference in error return sign can also be solved in the wrapper
> instead of with a nasty macro...
> 
> The compiler generates the same code, but it's a lot easier to read/review.

Interesting, sounds like food for thought for Eric Anholt. :)


-- 
Earthling Michel Dänzer      |     Debian (powerpc), X and DRI developer
Software libre enthusiast    |   http://svcs.affero.net/rm.php?r=daenzer

[-- Attachment #2: drm-nopage.diff --]
[-- Type: text/x-patch, Size: 6316 bytes --]

Index: drmP.h
===================================================================
RCS file: /cvs/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h,v
retrieving revision 1.85
diff -p -u -r1.85 drmP.h
--- drmP.h	5 Nov 2003 08:13:52 -0000	1.85
+++ drmP.h	1 Jan 2004 14:16:30 -0000
@@ -806,18 +819,34 @@ extern int	     DRM(flush)(struct file *
 extern int	     DRM(fasync)(int fd, struct file *filp, int on);
 
 				/* Mapping support (drm_vm.h) */
-extern struct page *DRM(vm_nopage)(struct vm_area_struct *vma,
-				   unsigned long address,
-				   int write_access);
-extern struct page *DRM(vm_shm_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_dma_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_sg_nopage)(struct vm_area_struct *vma,
-				      unsigned long address,
-				      int write_access);
+extern struct page   *DRM(vm_nopage)(struct vm_area_struct *vma,
+				     unsigned long address,
+				     int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+				     *
+#endif
+				     type);
+extern struct page   *DRM(vm_shm_nopage)(struct vm_area_struct *vma,
+					 unsigned long address,
+					 int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+					 *
+#endif
+					 type);
+extern struct page   *DRM(vm_dma_nopage)(struct vm_area_struct *vma,
+					 unsigned long address,
+					 int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+					 *
+#endif
+					 type);
+extern struct page   *DRM(vm_sg_nopage)(struct vm_area_struct *vma,
+					unsigned long address,
+					int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+					*
+#endif
+					type);
 extern void	     DRM(vm_open)(struct vm_area_struct *vma);
 extern void	     DRM(vm_close)(struct vm_area_struct *vma);
 extern void	     DRM(vm_shm_close)(struct vm_area_struct *vma);
Index: drm_vm.h
===================================================================
RCS file: /cvs/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_vm.h,v
retrieving revision 1.26
diff -p -u -r1.26 drm_vm.h
--- drm_vm.h	12 Sep 2003 20:00:59 -0000	1.26
+++ drm_vm.h	1 Jan 2004 14:16:30 -0000
@@ -69,7 +69,8 @@ struct vm_operations_struct   DRM(vm_sg_
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Find the right map and if it's AGP memory find the real physical page to
@@ -77,7 +78,11 @@ struct vm_operations_struct   DRM(vm_sg_
  */
 struct page *DRM(vm_nopage)(struct vm_area_struct *vma,
 			    unsigned long address,
-			    int write_access)
+			    int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+			    *
+#endif
+			    type)
 {
 #if __REALLY_HAVE_AGP
 	drm_file_t *priv  = vma->vm_file->private_data;
@@ -134,6 +139,10 @@ struct page *DRM(vm_nopage)(struct vm_ar
 			  baddr, __va(agpmem->memory->memory[offset]), offset,
 			  atomic_read(&page->count));
 
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+		if (type)
+			*type = VM_FAULT_MINOR;
+#endif
 		return page;
         }
 vm_nopage_error:
@@ -147,7 +156,8 @@ vm_nopage_error:
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Get the the mapping, find the real physical page to map, get the page, and
@@ -155,7 +165,11 @@ vm_nopage_error:
  */
 struct page *DRM(vm_shm_nopage)(struct vm_area_struct *vma,
 				unsigned long address,
-				int write_access)
+				int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+				*
+#endif
+				type)
 {
 	drm_map_t	 *map	 = (drm_map_t *)vma->vm_private_data;
 	unsigned long	 offset;
@@ -171,6 +185,10 @@ struct page *DRM(vm_shm_nopage)(struct v
 	if (!page)
 		return NOPAGE_OOM;
 	get_page(page);
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+	if (type)
+		*type = VM_FAULT_MINOR;
+#endif
 
 	DRM_DEBUG("shm_nopage 0x%lx\n", address);
 	return page;
@@ -262,14 +280,19 @@ void DRM(vm_shm_close)(struct vm_area_st
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Determine the page number from the page offset and get it from drm_device_dma::pagelist.
  */
 struct page *DRM(vm_dma_nopage)(struct vm_area_struct *vma,
 				unsigned long address,
-				int write_access)
+				int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+				*
+#endif
+				type)
 {
 	drm_file_t	 *priv	 = vma->vm_file->private_data;
 	drm_device_t	 *dev	 = priv->dev;
@@ -288,6 +311,10 @@ struct page *DRM(vm_dma_nopage)(struct v
 			     (offset & (~PAGE_MASK))));
 
 	get_page(page);
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+	if (type)
+		*type = VM_FAULT_MINOR;
+#endif
 
 	DRM_DEBUG("dma_nopage 0x%lx (page %lu)\n", address, page_nr);
 	return page;
@@ -298,14 +325,19 @@ struct page *DRM(vm_dma_nopage)(struct v
  *
  * \param vma virtual memory area.
  * \param address access address.
- * \param write_access sharing.
+ * \param type pointer to variable holding the type of fault
+ *        (unused before kernel 2.6.1).
  * \return pointer to the page structure.
  * 
  * Determine the map offset from the page offset and get it from drm_sg_mem::pagelist.
  */
 struct page *DRM(vm_sg_nopage)(struct vm_area_struct *vma,
 			       unsigned long address,
-			       int write_access)
+			       int
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+			       *
+#endif
+			       type)
 {
 	drm_map_t        *map    = (drm_map_t *)vma->vm_private_data;
 	drm_file_t *priv = vma->vm_file->private_data;
@@ -326,6 +358,10 @@ struct page *DRM(vm_sg_nopage)(struct vm
 	page_offset = (offset >> PAGE_SHIFT) + (map_offset >> PAGE_SHIFT);
 	page = entry->pagelist[page_offset];
 	get_page(page);
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
+	if (type)
+		*type = VM_FAULT_MINOR;
+#endif
 
 	return page;
 }

Re: [Dri-devel] 2.6 kernel change in nopage

[William Lee Irwin III]

On Thu, Jan 01, 2004 at 03:27:59PM +0100, Michel D?nzer wrote:
> Does this look better? Maybe a macro (or a typedef?) for the type of the
> last argument would still be a good idea? Or is there yet a better way?

I'm going to regret suggesting this, but how about:
(a) a typedef for the arg itself
(b) a macro and/or inline for the type update

both simultaneously?

So we'd have centralized in one place:

#if /* kernel version > 2.6.0 */
typdef int *third_arg_t;
#define third_arg_update(type)	do { *(type) = VM_FAULT_MINOR; } while (0)
#else
typdef int third_arg_t;
#define third_arg_update(type)	do { } while (0)
#endif

... and the natural usage that follows.

-- wli

Re: [Dri-devel] 2.6 kernel change in nopage

[Alan Cox]

On Iau, 2004-01-01 at 13:50, Michel Dänzer wrote:
> > Okay, you did something weird with nopage args, but I thought I did
> > the equivalent of this in the original patch?
> 
> This is about the canonical DRM code in the DRI tree.

99.9% of people run the DRM code in the kernel tree, so definitions of 
canonical might vary. I don't personally see a problem with the ifdefs.
The DRI devel tree has to work with anything, the kernel gets the luxury
of being able to strip out the defines that aren't needed for that
specific release.

Alan

Re: [Dri-devel] 2.6 kernel change in nopage

[Linus Torvalds]

On Thu, 1 Jan 2004, Michel Dänzer wrote:
>
> > well the advantage is that the ifdefs can just go away in kernel trees of
> > specific versions... (eg unifdef it)
> 
> Does this look better? Maybe a macro (or a typedef?) for the type of the
> last argument would still be a good idea? Or is there yet a better way?

My preference is either:

 - we can still undo the "nopage" argument change. It's been in the -mm 
   tree for a long time, and it _does_ solve a problem (page fault type 
   accounting), but the problem it solves is potentially so small that we
   might decide it's ok for 2.6.x.

   However, Andrew is king, and besides, it does fix a tiny bug, so I 
   don't think this is what we should do. I just wanted to put it on the 
   table as a possibility.

 - Use separate (and trivial) wrapper functions for this. Keep the "real" 
   function the same across everything, and just have a _static_ function 
   (ie no header file declaration crap) that does

   linux-new-vm.h:

	static int DRM(nopage_interface)(struct vm_area_struct * area, 
					 unsigned long address,
					 int *type)
	{
		*type = VM_FAULT_MINOR;
		DRM(nopage)(area, address);
	}


   linux-old-vm.h:

	static int DRM(nopage_interface)(struct vm_area_struct * area,
					 unsigned long address,   
					 int unused)
	{
		DRM(nopage)(area, address);
	}


   drm_vm.h:

	/*
	 * Or, poreferably, we could create a symlink and avoid
	 * the #if's at compile-time _entirely_.
	 */
	#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,0)
	  #include "linux-new-vm.h"
	#else
	  #include "linux-old-v.h
	#endif

	..
	.nopage = nopage_interface;
	..

Done right, the virtualization could be a bit higher still, and maybe 
the BSD code can do the same thing.

In general, at least I personally _much_ prefer the #ifdef's etc to be 
outside the code. So even if you don't like to have separate small files 
for different architectures/ports/versions, I'd at least personally 
much rather have

	#if xxxx

	int onewholefunction(..)
	...
	

	#else

	int onewholefunction(..)
	...

	#endif

rather than the messy

	int onewholefunction(
	#if xxx
	..
	#else
	..
	#endif
	(
	{
	...
	#if xxxx
	..
	#endif

The latter is a huge pain not just to read (you go blind after a while), 
but it's also painful as _hell_ to merge anywhere else.

In contrast, full-file interfaces for different kernel versions are a 
_lot_ easier to merge and keep track of. They may look like "duplication", 
but the advantages are legion. You don't mix different OS's and different 
versions together, and that makes it much easier to support them all 
without going crazy.

		Linus

Re: [Dri-devel] 2.6 kernel change in nopage

[Nigel Cunningham]

On Fri, 2004-01-02 at 09:19, Linus Torvalds wrote:
> In contrast, full-file interfaces for different kernel versions are a 
> _lot_ easier to merge and keep track of. They may look like "duplication", 
> but the advantages are legion. You don't mix different OS's and different 
> versions together, and that makes it much easier to support them all 
> without going crazy.

Of course there are also advantages to _not_ using the file-per-kernel
version scheme. Keeping one set of files means time is not wasted
applying the same change to multiple variations, removes the possibility
of patches getting applied to one version and not another and simplifies
the process of continuing to support old kernel versions. For merging, a
bit of test processing on the files could always be used to remove the
ugliness and clean things up.

Regards,

Nigel
-- 
My work on Software Suspend is graciously brought to you by
LinuxFund.org.

Re: [Dri-devel] 2.6 kernel change in nopage

[Linus Torvalds]

On Fri, 2 Jan 2004, Nigel Cunningham wrote:
> 
> Of course there are also advantages to _not_ using the file-per-kernel
> version scheme.

No there isn't.

The thing is, you should keep those "file-per-OS" files as small as 
possible, and only contain the things that are literally different. 
Because:

>		 Keeping one set of files means time is not wasted
> applying the same change to multiple variations

If the files only contain the actual differences, this just isn't an 
issue. Those files are per-OS _anyway_, so regardless of how you do it 
(with #ifdef's inside our outside the code etc), you'd have several 
versions.

And having separate files means that you don't uglify the code for another 
OS or another version and hide the _real_ issues.

But yes, it assumes that you can cleanly abstract out the differences.

		Linus

Re: [Dri-devel] 2.6 kernel change in nopage

[Michel Dänzer]

First of all, thanks for all the suggestions I've received in this
thread.

New patch up at http://penguinppc.org/~daenzer/DRI/drm-nopage.diff; does
this look acceptable to those who are going to do merges between the
trees? :)


-- 
Earthling Michel Dänzer      |     Debian (powerpc), X and DRI developer
Software libre enthusiast    |   http://svcs.affero.net/rm.php?r=daenzer

Re: [Dri-devel] 2.6 kernel change in nopage

[Arjan van de Ven]

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

On Sat, 2004-01-10 at 22:54, Michel Dänzer wrote:
> First of all, thanks for all the suggestions I've received in this
> thread.
> 
> New patch up at http://penguinppc.org/~daenzer/DRI/drm-nopage.diff; does
> this look acceptable to those who are going to do merges between the
> trees? :)

I like this one a whole lot better than the previous ones...
One could argue that you want the do_ function set the pagefault type
itself (and just igore the result in the 2.4 variants) but that's minor
nitpicking at most.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Dri-devel] 2.6 kernel change in nopage

[Michel Dänzer]

On Sat, 2004-01-10 at 23:08, Arjan van de Ven wrote:
> On Sat, 2004-01-10 at 22:54, Michel Dänzer wrote:
> > First of all, thanks for all the suggestions I've received in this
> > thread.
> > 
> > New patch up at http://penguinppc.org/~daenzer/DRI/drm-nopage.diff; does
> > this look acceptable to those who are going to do merges between the
> > trees? :)
> 
> I like this one a whole lot better than the previous ones...
> One could argue that you want the do_ function set the pagefault type
> itself (and just igore the result in the 2.4 variants) but that's minor
> nitpicking at most.

Okay, thanks, I just committed this.


-- 
Earthling Michel Dänzer      |     Debian (powerpc), X and DRI developer
Software libre enthusiast    |   http://svcs.affero.net/rm.php?r=daenzer