[PATCH] video: arch specific page protection support for deferred io

[PATCH] video: arch specific page protection support for deferred io

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

This patch adds arch specific page protection support to deferred io.

Instead of overwriting the info->fbops->mmap pointer with the
deferred io specific mmap callback, modify fb_mmap() to include
a #ifdef wrapped call to fb_deferred_io_mmap().  The function
fb_deferred_io_mmap() is extended to call fb_pgprotect() in the
case of non-vmalloc() frame buffers.

With this patch uncached deferred io can be used together with
the sh_mobile_lcdcfb driver. Without this patch arch specific
page protection code in fb_pgprotect() never gets invoked with
deferred io.

Signed-off-by: Magnus Damm <damm@igel.co.jp>
---

 For proper runtime operation with uncached vmas make sure
 "[PATCH][RFC] mm: uncached vma support with writenotify"
 is applied. There are no merge order dependencies.

 drivers/video/fb_defio.c |   10 +++++++---
 drivers/video/fbmem.c    |    6 ++++++
 include/linux/fb.h       |    2 ++
 3 files changed, 15 insertions(+), 3 deletions(-)

--- 0001/drivers/video/fb_defio.c
+++ work/drivers/video/fb_defio.c	2009-06-24 19:07:11.000000000 +0900
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/fb.h>
 #include <linux/list.h>
+#include <asm/fb.h>
 
 /* to support deferred IO */
 #include <linux/rmap.h>
@@ -141,11 +142,16 @@ static const struct address_space_operat
 	.set_page_dirty = fb_deferred_io_set_page_dirty,
 };
 
-static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
+int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
+			struct vm_area_struct *vma, unsigned long off)
 {
 	vma->vm_ops = &fb_deferred_io_vm_ops;
 	vma->vm_flags |= ( VM_IO | VM_RESERVED | VM_DONTEXPAND );
 	vma->vm_private_data = info;
+
+	if (!is_vmalloc_addr(info->screen_base))
+		fb_pgprotect(file, vma, off);
+
 	return 0;
 }
 
@@ -182,7 +188,6 @@ void fb_deferred_io_init(struct fb_info 
 
 	BUG_ON(!fbdefio);
 	mutex_init(&fbdefio->lock);
-	info->fbops->fb_mmap = fb_deferred_io_mmap;
 	INIT_DELAYED_WORK(&info->deferred_work, fb_deferred_io_work);
 	INIT_LIST_HEAD(&fbdefio->pagelist);
 	if (fbdefio->delay == 0) /* set a default of 1 s */
@@ -214,7 +219,6 @@ void fb_deferred_io_cleanup(struct fb_in
 		page->mapping = NULL;
 	}
 
-	info->fbops->fb_mmap = NULL;
 	mutex_destroy(&fbdefio->lock);
 }
 EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);
--- 0001/drivers/video/fbmem.c
+++ work/drivers/video/fbmem.c	2009-06-24 19:12:29.000000000 +0900
@@ -1325,6 +1325,12 @@ __releases(&info->lock)
 	off = vma->vm_pgoff << PAGE_SHIFT;
 	if (!fb)
 		return -ENODEV;
+
+#ifdef CONFIG_FB_DEFERRED_IO
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(file, info, vma, off);
+#endif
+
 	if (fb->fb_mmap) {
 		int res;
 		mutex_lock(&info->lock);
--- 0001/include/linux/fb.h
+++ work/include/linux/fb.h	2009-06-24 19:04:49.000000000 +0900
@@ -1005,6 +1005,8 @@ extern void fb_deferred_io_open(struct f
 extern void fb_deferred_io_cleanup(struct fb_info *info);
 extern int fb_deferred_io_fsync(struct file *file, struct dentry *dentry,
 				int datasync);
+extern int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
+			       struct vm_area_struct *vma, unsigned long off);
 
 static inline bool fb_be_math(struct fb_info *info)
 {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Andrew Morton]

On Wed, 24 Jun 2009 19:54:13 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:

> From: Magnus Damm <damm@igel.co.jp>
> 
> This patch adds arch specific page protection support to deferred io.
> 
> Instead of overwriting the info->fbops->mmap pointer with the
> deferred io specific mmap callback, modify fb_mmap() to include
> a #ifdef wrapped call to fb_deferred_io_mmap().  The function
> fb_deferred_io_mmap() is extended to call fb_pgprotect() in the
> case of non-vmalloc() frame buffers.
> 
> With this patch uncached deferred io can be used together with
> the sh_mobile_lcdcfb driver. Without this patch arch specific
> page protection code in fb_pgprotect() never gets invoked with
> deferred io.
> 
> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> ---
> 
>  For proper runtime operation with uncached vmas make sure
>  "[PATCH][RFC] mm: uncached vma support with writenotify"
>  is applied. There are no merge order dependencies.

So this is dependent upon a patch which is in your tree, which is in
linux-next?

Tricky.  Perhaps we should merge this via your tree, should it survive
review.


>  drivers/video/fb_defio.c |   10 +++++++---
>  drivers/video/fbmem.c    |    6 ++++++
>  include/linux/fb.h       |    2 ++
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> --- 0001/drivers/video/fb_defio.c
> +++ work/drivers/video/fb_defio.c	2009-06-24 19:07:11.000000000 +0900
> @@ -19,6 +19,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/fb.h>
>  #include <linux/list.h>
> +#include <asm/fb.h>

Microblaze doesn't have an asm/fb.h.

>  /* to support deferred IO */
>  #include <linux/rmap.h>
> @@ -141,11 +142,16 @@ static const struct address_space_operat
>  	.set_page_dirty = fb_deferred_io_set_page_dirty,
>  };
>  
> -static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
> +			struct vm_area_struct *vma, unsigned long off)
>  {
>  	vma->vm_ops = &fb_deferred_io_vm_ops;
>  	vma->vm_flags |= ( VM_IO | VM_RESERVED | VM_DONTEXPAND );
>  	vma->vm_private_data = info;
> +
> +	if (!is_vmalloc_addr(info->screen_base))
> +		fb_pgprotect(file, vma, off);

Add a comment explaining what's going on here?

>  	return 0;
>  }
>  
> @@ -182,7 +188,6 @@ void fb_deferred_io_init(struct fb_info 
>  
>  	BUG_ON(!fbdefio);
>  	mutex_init(&fbdefio->lock);
> -	info->fbops->fb_mmap = fb_deferred_io_mmap;
>  	INIT_DELAYED_WORK(&info->deferred_work, fb_deferred_io_work);
>  	INIT_LIST_HEAD(&fbdefio->pagelist);
>  	if (fbdefio->delay == 0) /* set a default of 1 s */
> @@ -214,7 +219,6 @@ void fb_deferred_io_cleanup(struct fb_in
>  		page->mapping = NULL;
>  	}
>  
> -	info->fbops->fb_mmap = NULL;
>  	mutex_destroy(&fbdefio->lock);
>  }
>  EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);
> --- 0001/drivers/video/fbmem.c
> +++ work/drivers/video/fbmem.c	2009-06-24 19:12:29.000000000 +0900
> @@ -1325,6 +1325,12 @@ __releases(&info->lock)
>  	off = vma->vm_pgoff << PAGE_SHIFT;
>  	if (!fb)
>  		return -ENODEV;
> +
> +#ifdef CONFIG_FB_DEFERRED_IO
> +	if (info->fbdefio)
> +		return fb_deferred_io_mmap(file, info, vma, off);
> +#endif

We can remove the ifdefs here...

> +extern int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
> +			       struct vm_area_struct *vma, unsigned long off);

if we do

#else	/* CONFIG_FB_DEFERRED_IO */
static inline int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
				struct vm_area_struct *vma, unsigned long off)
{
	return 0;
}
#endif	/* CONFIG_FB_DEFERRED_IO */

here.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Paul Mundt]

On Wed, Jun 24, 2009 at 07:56:47PM -0700, Andrew Morton wrote:
> On Wed, 24 Jun 2009 19:54:13 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
> 
> > From: Magnus Damm <damm@igel.co.jp>
> > 
> > This patch adds arch specific page protection support to deferred io.
> > 
> > Instead of overwriting the info->fbops->mmap pointer with the
> > deferred io specific mmap callback, modify fb_mmap() to include
> > a #ifdef wrapped call to fb_deferred_io_mmap().  The function
> > fb_deferred_io_mmap() is extended to call fb_pgprotect() in the
> > case of non-vmalloc() frame buffers.
> > 
> > With this patch uncached deferred io can be used together with
> > the sh_mobile_lcdcfb driver. Without this patch arch specific
> > page protection code in fb_pgprotect() never gets invoked with
> > deferred io.
> > 
> > Signed-off-by: Magnus Damm <damm@igel.co.jp>
> > ---
> > 
> >  For proper runtime operation with uncached vmas make sure
> >  "[PATCH][RFC] mm: uncached vma support with writenotify"
> >  is applied. There are no merge order dependencies.
> 
> So this is dependent upon a patch which is in your tree, which is in
> linux-next?
> 
This patch is not in the sh tree, either, we wanted it to go via -mm, but
it has the issue that it depends on pgprot_noncached() being generally
defined, so there is a bit of an ordering mess. It could be re-posted
with an ifdef pgprot_noncached to get it merged while we wait for the
outstanding architectures to catch up, and this is indeed what the bulk
of the in-tree pgprot_noncached() users in generic places end up doing
already.

Of course I can take both through the sh tree once people are happy with
the patches.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Andrew Morton]

On Thu, 25 Jun 2009 12:09:34 +0900 Paul Mundt <lethal@linux-sh.org> wrote:

> On Wed, Jun 24, 2009 at 07:56:47PM -0700, Andrew Morton wrote:
> > On Wed, 24 Jun 2009 19:54:13 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
> > 
> > > From: Magnus Damm <damm@igel.co.jp>
> > > 
> > > This patch adds arch specific page protection support to deferred io.
> > > 
> > > Instead of overwriting the info->fbops->mmap pointer with the
> > > deferred io specific mmap callback, modify fb_mmap() to include
> > > a #ifdef wrapped call to fb_deferred_io_mmap().  The function
> > > fb_deferred_io_mmap() is extended to call fb_pgprotect() in the
> > > case of non-vmalloc() frame buffers.
> > > 
> > > With this patch uncached deferred io can be used together with
> > > the sh_mobile_lcdcfb driver. Without this patch arch specific
> > > page protection code in fb_pgprotect() never gets invoked with
> > > deferred io.
> > > 
> > > Signed-off-by: Magnus Damm <damm@igel.co.jp>
> > > ---
> > > 
> > >  For proper runtime operation with uncached vmas make sure
> > >  "[PATCH][RFC] mm: uncached vma support with writenotify"
> > >  is applied. There are no merge order dependencies.
> > 
> > So this is dependent upon a patch which is in your tree, which is in
> > linux-next?
> > 
> This patch is not in the sh tree, either, we wanted it to go via -mm, but
> it has the issue that it depends on pgprot_noncached() being generally
> defined, so there is a bit of an ordering mess. It could be re-posted
> with an ifdef pgprot_noncached to get it merged while we wait for the
> outstanding architectures to catch up, and this is indeed what the bulk
> of the in-tree pgprot_noncached() users in generic places end up doing
> already.
> 
> Of course I can take both through the sh tree once people are happy with
> the patches.

That's OK by me - the patches are small and there's not much point in
going all formal over who-does-which-bit.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Magnus Damm]

On Thu, Jun 25, 2009 at 11:56 AM, Andrew
Morton<akpm@linux-foundation.org> wrote:
> On Wed, 24 Jun 2009 19:54:13 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
>
>> From: Magnus Damm <damm@igel.co.jp>
>>
>> This patch adds arch specific page protection support to deferred io.
>>
>> Instead of overwriting the info->fbops->mmap pointer with the
>> deferred io specific mmap callback, modify fb_mmap() to include
>> a #ifdef wrapped call to fb_deferred_io_mmap().  The function
>> fb_deferred_io_mmap() is extended to call fb_pgprotect() in the
>> case of non-vmalloc() frame buffers.
>>
>> With this patch uncached deferred io can be used together with
>> the sh_mobile_lcdcfb driver. Without this patch arch specific
>> page protection code in fb_pgprotect() never gets invoked with
>> deferred io.
>>
>> Signed-off-by: Magnus Damm <damm@igel.co.jp>
>> ---
>>
>>  For proper runtime operation with uncached vmas make sure
>>  "[PATCH][RFC] mm: uncached vma support with writenotify"
>>  is applied. There are no merge order dependencies.
>
> So this is dependent upon a patch which is in your tree, which is in
> linux-next?

I tried to say that there were _no_ dependencies merge wise. =)

There are 3 levels of dependencies:
1: pgprot_noncached() patches from Arnd
2: mm: uncached vma support with writenotify
3: video: arch specfic page protection support for deferred io

2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
1. So this patch can be merged independently.

>
>>  drivers/video/fb_defio.c |   10 +++++++---
>>  drivers/video/fbmem.c    |    6 ++++++
>>  include/linux/fb.h       |    2 ++
>>  3 files changed, 15 insertions(+), 3 deletions(-)
>>
>> --- 0001/drivers/video/fb_defio.c
>> +++ work/drivers/video/fb_defio.c     2009-06-24 19:07:11.000000000 +0900
>> @@ -19,6 +19,7 @@
>>  #include <linux/interrupt.h>
>>  #include <linux/fb.h>
>>  #include <linux/list.h>
>> +#include <asm/fb.h>
>
> Microblaze doesn't have an asm/fb.h.

Right, but fbmem.c includes asm/fb.h as well (for fb_pgprotect()), so
framebuffer isn't supported on Microblaze. So I think this is a
separate issue.

>>  /* to support deferred IO */
>>  #include <linux/rmap.h>
>> @@ -141,11 +142,16 @@ static const struct address_space_operat
>>       .set_page_dirty = fb_deferred_io_set_page_dirty,
>>  };
>>
>> -static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>> +int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
>> +                     struct vm_area_struct *vma, unsigned long off)
>>  {
>>       vma->vm_ops = &fb_deferred_io_vm_ops;
>>       vma->vm_flags |= ( VM_IO | VM_RESERVED | VM_DONTEXPAND );
>>       vma->vm_private_data = info;
>> +
>> +     if (!is_vmalloc_addr(info->screen_base))
>> +             fb_pgprotect(file, vma, off);
>
> Add a comment explaining what's going on here?

Good idea!

>> @@ -1325,6 +1325,12 @@ __releases(&info->lock)
>>       off = vma->vm_pgoff << PAGE_SHIFT;
>>       if (!fb)
>>               return -ENODEV;
>> +
>> +#ifdef CONFIG_FB_DEFERRED_IO
>> +     if (info->fbdefio)
>> +             return fb_deferred_io_mmap(file, info, vma, off);
>> +#endif
>
> We can remove the ifdefs here...
>
>> +extern int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
>> +                            struct vm_area_struct *vma, unsigned long off);
>
> if we do
>
> #else   /* CONFIG_FB_DEFERRED_IO */
> static inline int fb_deferred_io_mmap(struct file *file, struct fb_info *info,
>                                struct vm_area_struct *vma, unsigned long off)
> {
>        return 0;
> }
> #endif  /* CONFIG_FB_DEFERRED_IO */
>
> here.

The code is fbmem.c is currently filled with #ifdefs today, want me
create inline versions for fb_deferred_io_open() and
fb_deferred_io_fsync() as well?

Thanks for your comments!

/ magnus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Andrew Morton]

On Thu, 25 Jun 2009 15:06:24 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:

> On Thu, Jun 25, 2009 at 11:56 AM, Andrew
> Morton<akpm@linux-foundation.org> wrote:
> > On Wed, 24 Jun 2009 19:54:13 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
> >
> >> From: Magnus Damm <damm@igel.co.jp>
> >>
> >> This patch adds arch specific page protection support to deferred io.
> >>
> >> Instead of overwriting the info->fbops->mmap pointer with the
> >> deferred io specific mmap callback, modify fb_mmap() to include
> >> a #ifdef wrapped call to fb_deferred_io_mmap(). __The function
> >> fb_deferred_io_mmap() is extended to call fb_pgprotect() in the
> >> case of non-vmalloc() frame buffers.
> >>
> >> With this patch uncached deferred io can be used together with
> >> the sh_mobile_lcdcfb driver. Without this patch arch specific
> >> page protection code in fb_pgprotect() never gets invoked with
> >> deferred io.
> >>
> >> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> >> ---
> >>
> >> __For proper runtime operation with uncached vmas make sure
> >> __"[PATCH][RFC] mm: uncached vma support with writenotify"
> >> __is applied. There are no merge order dependencies.
> >
> > So this is dependent upon a patch which is in your tree, which is in
> > linux-next?
> 
> I tried to say that there were _no_ dependencies merge wise. =)
>
> There are 3 levels of dependencies:
> 1: pgprot_noncached() patches from Arnd
> 2: mm: uncached vma support with writenotify
> 3: video: arch specfic page protection support for deferred io
> 
> 2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
> 1. So this patch can be merged independently.

OIC.  I didn't like the idea of improper runtime operation ;)

Still, it's messy.  If only because various trees might be running
untested combinations of patches.  Can we get these all into the same
tree?  Paul's?

> 
> The code is fbmem.c is currently filled with #ifdefs today, want me
> create inline versions for fb_deferred_io_open() and
> fb_deferred_io_fsync() as well?

It was a minor point.  Your call.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Paul Mundt]

On Thu, Jun 25, 2009 at 12:03:59AM -0700, Andrew Morton wrote:
> On Thu, 25 Jun 2009 15:06:24 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
> > There are 3 levels of dependencies:
> > 1: pgprot_noncached() patches from Arnd
> > 2: mm: uncached vma support with writenotify
> > 3: video: arch specfic page protection support for deferred io
> > 
> > 2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
> > 1. So this patch can be merged independently.
> 
> OIC.  I didn't like the idea of improper runtime operation ;)
> 
> Still, it's messy.  If only because various trees might be running
> untested combinations of patches.  Can we get these all into the same
> tree?  Paul's?
> 
#1 is a bit tricky. cris has already merged the pgprot_noncached() patch,
which means only m32r and xtensa are outstanding, and unfortunately
neither one of those is very fast to pick up changes. OTOH, both of those
do include asm-generic/pgtable.h, so the build shouldn't break in -next
for those two if I merge #2 and #3, even if the behaviour won't be
correct for those platforms until they merge their pgprot_noncached()
patches (I think this is ok, since it's not changing any behaviour they
experience today anyways).

It would be nice to have an ack from someone for #2 before merging it,
but it's been out there long enough that people have had ample time to
raise objections.

So I'll make this the last call for acks or nacks on #2 and #3, if none
show up in the next couple of days, I'll fold them in to my tree and
they'll show up in -next starting next week.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Andrew Morton]

On Fri, 26 Jun 2009 02:38:06 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Thu, Jun 25, 2009 at 12:03:59AM -0700, Andrew Morton wrote:
> > On Thu, 25 Jun 2009 15:06:24 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
> > > There are 3 levels of dependencies:
> > > 1: pgprot_noncached() patches from Arnd
> > > 2: mm: uncached vma support with writenotify
> > > 3: video: arch specfic page protection support for deferred io
> > > 
> > > 2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
> > > 1. So this patch can be merged independently.
> > 
> > OIC.  I didn't like the idea of improper runtime operation ;)
> > 
> > Still, it's messy.  If only because various trees might be running
> > untested combinations of patches.  Can we get these all into the same
> > tree?  Paul's?
> > 
> #1 is a bit tricky. cris has already merged the pgprot_noncached() patch,
> which means only m32r and xtensa are outstanding, and unfortunately
> neither one of those is very fast to pick up changes. OTOH, both of those
> do include asm-generic/pgtable.h, so the build shouldn't break in -next
> for those two if I merge #2 and #3, even if the behaviour won't be
> correct for those platforms until they merge their pgprot_noncached()
> patches (I think this is ok, since it's not changing any behaviour they
> experience today anyways).
> 
> It would be nice to have an ack from someone for #2 before merging it,
> but it's been out there long enough that people have had ample time to
> raise objections.
> 
> So I'll make this the last call for acks or nacks on #2 and #3, if none
> show up in the next couple of days, I'll fold them in to my tree and
> they'll show up in -next starting next week.

Well my head span off ages ago.  Could someone please resend all three
patches?

<hunts around and finds #2>

I don't really understand that one.  Have we heard fro Jaya recently?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Jaya Kumar]

On Fri, Jun 26, 2009 at 2:12 AM, Andrew Morton<akpm@linux-foundation.org> wrote:
> On Fri, 26 Jun 2009 02:38:06 +0900
> Paul Mundt <lethal@linux-sh.org> wrote:
>
>> On Thu, Jun 25, 2009 at 12:03:59AM -0700, Andrew Morton wrote:
>> > On Thu, 25 Jun 2009 15:06:24 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
>> > > There are 3 levels of dependencies:
>> > > 1: pgprot_noncached() patches from Arnd
>> > > 2: mm: uncached vma support with writenotify
>> > > 3: video: arch specfic page protection support for deferred io
>> > >
>> > > 2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
>> > > 1. So this patch can be merged independently.
> <hunts around and finds #2>
>
> I don't really understand that one.  Have we heard fro Jaya recently?
>

He's been having some personal problems so he's been quiet. :-)

Magnus's defio changes, #3 look fine to me. I don't know much about #2
but what I understood was that the previous mmap_region code was
unintentionally turning caching back on when it did the writenotify
test and I guess Magnus's goal with the patch is to make sure that
address range is left uncached since sh_mobile uses DMA to transfer
the framebuffer and he might have encountered coherency issues there?

Thanks,
jaya

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Paul Mundt]

On Fri, Jun 26, 2009 at 02:36:23AM +0800, Jaya Kumar wrote:
> On Fri, Jun 26, 2009 at 2:12 AM, Andrew Morton<akpm@linux-foundation.org> wrote:
> > On Fri, 26 Jun 2009 02:38:06 +0900
> > Paul Mundt <lethal@linux-sh.org> wrote:
> >
> >> On Thu, Jun 25, 2009 at 12:03:59AM -0700, Andrew Morton wrote:
> >> > On Thu, 25 Jun 2009 15:06:24 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
> >> > > There are 3 levels of dependencies:
> >> > > 1: pgprot_noncached() patches from Arnd
> >> > > 2: mm: uncached vma support with writenotify
> >> > > 3: video: arch specfic page protection support for deferred io
> >> > >
> >> > > 2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
> >> > > 1. So this patch can be merged independently.
> > <hunts around and finds #2>
> >
> > I don't really understand that one. ?Have we heard fro Jaya recently?
> >
> 
> He's been having some personal problems so he's been quiet. :-)
> 
> Magnus's defio changes, #3 look fine to me. I don't know much about #2
> but what I understood was that the previous mmap_region code was
> unintentionally turning caching back on when it did the writenotify
> test and I guess Magnus's goal with the patch is to make sure that
> address range is left uncached since sh_mobile uses DMA to transfer
> the framebuffer and he might have encountered coherency issues there?
> 
Correct. This could have been documented a bit better, but yes, the issue
is that when the writenotify test kicks in the vma silently loses its
uncachedness which leads to the aforementioned runtime behaviour issues.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Magnus Damm]

On Fri, Jun 26, 2009 at 3:12 AM, Andrew Morton<akpm@linux-foundation.org> wrote:
> On Fri, 26 Jun 2009 02:38:06 +0900
> Paul Mundt <lethal@linux-sh.org> wrote:
>
>> On Thu, Jun 25, 2009 at 12:03:59AM -0700, Andrew Morton wrote:
>> > On Thu, 25 Jun 2009 15:06:24 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
>> > > There are 3 levels of dependencies:
>> > > 1: pgprot_noncached() patches from Arnd
>> > > 2: mm: uncached vma support with writenotify
>> > > 3: video: arch specfic page protection support for deferred io
>> > >
>> > > 2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
>> > > 1. So this patch can be merged independently.
>> >
>> > OIC.  I didn't like the idea of improper runtime operation ;)
>> >
>> > Still, it's messy.  If only because various trees might be running
>> > untested combinations of patches.  Can we get these all into the same
>> > tree?  Paul's?
>> >
>> #1 is a bit tricky. cris has already merged the pgprot_noncached() patch,
>> which means only m32r and xtensa are outstanding, and unfortunately
>> neither one of those is very fast to pick up changes. OTOH, both of those
>> do include asm-generic/pgtable.h, so the build shouldn't break in -next
>> for those two if I merge #2 and #3, even if the behaviour won't be
>> correct for those platforms until they merge their pgprot_noncached()
>> patches (I think this is ok, since it's not changing any behaviour they
>> experience today anyways).
>>
>> It would be nice to have an ack from someone for #2 before merging it,
>> but it's been out there long enough that people have had ample time to
>> raise objections.
>>
>> So I'll make this the last call for acks or nacks on #2 and #3, if none
>> show up in the next couple of days, I'll fold them in to my tree and
>> they'll show up in -next starting next week.
>
> Well my head span off ages ago.  Could someone please resend all three
> patches?
>
> <hunts around and finds #2>
>
> I don't really understand that one.  Have we heard fro Jaya recently?

Some f_op->mmap() callbacks invoked from mmap_region() may want to use
writenotify but also modify vma->vm_page_prot to for instance mark the
vma as uncached.

Without patch #2 the vma->vm_page_prot value set by f_op->mmap() gets
overwritten if writenotify is enabled. So in the case of writenotify
the vma will never be uncached even though f_op->mmap() marks it as
such. Patch #2 makes it possible to keep the uncached setting made by
f_op->mmap() and use writenotify.

On SuperH we want to use deferred io with an uncached vma, so patch #3
makes sure our arch specific fb_pgprotect() function gets called so we
can mark the vma as uncached.

Hope this clarifies a bit.

/ magnus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Magnus Damm]

On Thu, Jun 25, 2009 at 4:03 PM, Andrew Morton<akpm@linux-foundation.org> wrote:
> On Thu, 25 Jun 2009 15:06:24 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
>
>> On Thu, Jun 25, 2009 at 11:56 AM, Andrew
>> Morton<akpm@linux-foundation.org> wrote:
>> > On Wed, 24 Jun 2009 19:54:13 +0900 Magnus Damm <magnus.damm@gmail.com> wrote:
>> >
>> >> From: Magnus Damm <damm@igel.co.jp>
>> >>
>> >> This patch adds arch specific page protection support to deferred io.
>> >>
>> >> Instead of overwriting the info->fbops->mmap pointer with the
>> >> deferred io specific mmap callback, modify fb_mmap() to include
>> >> a #ifdef wrapped call to fb_deferred_io_mmap(). __The function
>> >> fb_deferred_io_mmap() is extended to call fb_pgprotect() in the
>> >> case of non-vmalloc() frame buffers.
>> >>
>> >> With this patch uncached deferred io can be used together with
>> >> the sh_mobile_lcdcfb driver. Without this patch arch specific
>> >> page protection code in fb_pgprotect() never gets invoked with
>> >> deferred io.
>> >>
>> >> Signed-off-by: Magnus Damm <damm@igel.co.jp>
>> >> ---
>> >>
>> >> __For proper runtime operation with uncached vmas make sure
>> >> __"[PATCH][RFC] mm: uncached vma support with writenotify"
>> >> __is applied. There are no merge order dependencies.
>> >
>> > So this is dependent upon a patch which is in your tree, which is in
>> > linux-next?
>>
>> I tried to say that there were _no_ dependencies merge wise. =)
>>
>> There are 3 levels of dependencies:
>> 1: pgprot_noncached() patches from Arnd
>> 2: mm: uncached vma support with writenotify
>> 3: video: arch specfic page protection support for deferred io
>>
>> 2 depends on 1 to compile, but 3 (this one) is disconnected from 2 and
>> 1. So this patch can be merged independently.
>
> OIC.  I didn't like the idea of improper runtime operation ;)
>
> Still, it's messy.  If only because various trees might be running
> untested combinations of patches.  Can we get these all into the same
> tree?  Paul's?

There may also be some dependencies related to other patches posted to
linux-fbdev-devel, for instance:
[PATCH] add mutex to fbdev for fb_mmap locking (v2)
The fb_mmap() locking will conflict with this patch.

So it may make sense to group the fbdev patches together.

>>
>> The code is fbmem.c is currently filled with #ifdefs today, want me
>> create inline versions for fb_deferred_io_open() and
>> fb_deferred_io_fsync() as well?
>
> It was a minor point.  Your call.

I'd prefer to submit a patch on top of this one if possible.

Cheers,

/ magnus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Jaya Kumar]

On Thu, Jun 25, 2009 at 2:06 PM, Magnus Damm<magnus.damm@gmail.com> wrote:
>
> The code is fbmem.c is currently filled with #ifdefs today, want me
> create inline versions for fb_deferred_io_open() and
> fb_deferred_io_fsync() as well?
>

The patch looks good. I was going to suggest that it might be
attractive to use __attribute__(weak) for each of the dummy functions
instead of ifdefs in this case, but I can't remember if there was a
consensus about attribute-weak versus ifdefs.

Thanks,
jaya

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] video: arch specific page protection support for deferred io

[Arnd Bergmann]

On Thursday 25 June 2009, Jaya Kumar wrote:
> The patch looks good. I was going to suggest that it might be
> attractive to use __attribute__(weak) for each of the dummy functions
> instead of ifdefs in this case, but I can't remember if there was a
> consensus about attribute-weak versus ifdefs.

We rarely use weak functions, the canonical way to express an optional
subsystem is along the lines of

/* include/linux/foo.h */
#ifdef CONFIG_FOO
extern int bar(void);
#else
static inline int bar(void)
{
	return 0;
}
#endif
---
/* foo/foo.c */
int bar(void)
{
	/* the real thing here */
	...
}
---
# foo/Makefile 
obj-$(CONFIG_FOO) += foo.c

Most uses of __weak or __attribute__((weak)) are for working default
implementations that can be overridden by architecture specific code.
However, for these the preferred way to express seems to have shifted
towards variations of:

/* include/linux/foo.h */
#include <asm/foo.h>
#ifndef bar
static inline int bar(void)
{
	/* generic implementation */
	...
}
#endif
/* arch/*/include/asm/foo.h */
#define bar bar
static inline int bar(void)
{
	/* arch specific implementation */
	...
}

	Arnd <><

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>