Re: [patch 07/18] v4l: nopage
On Fri, 7 Dec 2007 16:31:42 -0800 Andrew Morton wrote:
> On Wed, 05 Dec 2007 18:15:54 +1100
> [EMAIL PROTECTED] wrote:
>
> > +static int
> > +videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> > {
> > struct page *page;
> >
> > - dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
> > - vaddr,vma->vm_start,vma->vm_end);
> > - if (vaddr > vma->vm_end)
> > - return NOPAGE_SIGBUS;
> > + dprintk(3,"fault: fault @ %08lx [vma %08lx-%08lx]\n",
> > + (unsigned long)vmf->virtual_address,vma->vm_start,vma->vm_end);
> > page = alloc_page(GFP_USER | __GFP_DMA32);
> > if (!page)
> > - return NOPAGE_OOM;
> > + return VM_FAULT_OOM;
> > clear_user_page(page_address(page), vaddr, page);
>
> This didn't compile on sparc64 because `vaddr' is undefined.
>
>
> Let us see why:
>
> #define clear_user_page(page, vaddr, pg) clear_page(page)
> #define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
>
> #define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
> alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
>
> root cause: lack of argument checking on x86 due to stupid macros.
Hm, I would have said that the root cause was that Nick removed the
vaddr parameter from the function parameters:
-static struct page*
-videobuf_vm_nopage(struct vm_area_struct *vma, unsigned long vaddr,
- int *type)
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
Should the function now produce 'vaddr' internally?
> Could someone *please* start a little project of extirpating this utter
> brain damage? Convert those macros to typechecked static inlines on x86
> (at least) so this sort of thing (which happens again and again and again)
> is lessened?
Yep, unless someone else is already doing so...
> macros are such miserable things. I wonder if we could get checkpatch to
> help out here?
---
~Randy
Features and documentation: http://lwn.net/Articles/260136/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 07/18] v4l: nopage
On Fri, 7 Dec 2007 16:31:42 -0800 Andrew Morton wrote:
On Wed, 05 Dec 2007 18:15:54 +1100
[EMAIL PROTECTED] wrote:
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct page *page;
- dprintk(3,nopage: fault @ %08lx [vma %08lx-%08lx]\n,
- vaddr,vma-vm_start,vma-vm_end);
- if (vaddr vma-vm_end)
- return NOPAGE_SIGBUS;
+ dprintk(3,fault: fault @ %08lx [vma %08lx-%08lx]\n,
+ (unsigned long)vmf-virtual_address,vma-vm_start,vma-vm_end);
page = alloc_page(GFP_USER | __GFP_DMA32);
if (!page)
- return NOPAGE_OOM;
+ return VM_FAULT_OOM;
clear_user_page(page_address(page), vaddr, page);
This didn't compile on sparc64 because `vaddr' is undefined.
Let us see why:
#define clear_user_page(page, vaddr, pg) clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
root cause: lack of argument checking on x86 due to stupid macros.
Hm, I would have said that the root cause was that Nick removed the
vaddr parameter from the function parameters:
-static struct page*
-videobuf_vm_nopage(struct vm_area_struct *vma, unsigned long vaddr,
- int *type)
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
Should the function now produce 'vaddr' internally?
Could someone *please* start a little project of extirpating this utter
brain damage? Convert those macros to typechecked static inlines on x86
(at least) so this sort of thing (which happens again and again and again)
is lessened?
Yep, unless someone else is already doing so...
macros are such miserable things. I wonder if we could get checkpatch to
help out here?
---
~Randy
Features and documentation: http://lwn.net/Articles/260136/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 07/18] v4l: nopage
On Sat, 8 Dec 2007 10:15:08 +0100 Ingo Molnar <[EMAIL PROTECTED]> wrote:
>
> * Andrew Morton <[EMAIL PROTECTED]> wrote:
>
> > On Wed, 05 Dec 2007 18:15:54 +1100
> > [EMAIL PROTECTED] wrote:
> >
> > > +static int
> > > +videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> > > {
> > > struct page *page;
> > >
> > > - dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
> > > - vaddr,vma->vm_start,vma->vm_end);
> > > - if (vaddr > vma->vm_end)
> > > - return NOPAGE_SIGBUS;
> > > + dprintk(3,"fault: fault @ %08lx [vma %08lx-%08lx]\n",
> > > + (unsigned long)vmf->virtual_address,vma->vm_start,vma->vm_end);
> > > page = alloc_page(GFP_USER | __GFP_DMA32);
> > > if (!page)
> > > - return NOPAGE_OOM;
> > > + return VM_FAULT_OOM;
> > > clear_user_page(page_address(page), vaddr, page);
> >
> > This didn't compile on sparc64 because `vaddr' is undefined.
> >
> >
> > Let us see why:
> >
> > #define clear_user_page(page, vaddr, pg)clear_page(page)
> > #define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
> >
> > #define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
> > alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
> >
> > root cause: lack of argument checking on x86 due to stupid macros.
> >
> >
> > Could someone *please* start a little project of extirpating this
> > utter brain damage? Convert those macros to typechecked static
> > inlines on x86 (at least) so this sort of thing (which happens again
> > and again and again) is lessened?
We should fix existing stuff, like this.
> i wanted to write a reply to suggest a checkpatch policy for this. When
> i noticed this sentence at the end of your mail:
>
> > macros are such miserable things. I wonder if we could get checkpatch
> > to help out here?
>
> /me too :-)
>
> any policy that gets into checkpatch.pl's default output is a policy for
> arch/x86 patch merging. (as long as it's not a false positive) And
> because we do all these unifications the effects of checkpatch.pl
> permeate basically every aspect of arch/x86.
>
> one approach would be to make new macros in include/* a no-no. That
> would hurt a few of the legitimate uses though, so maybe a useful
> refinement would be to check the structure of macros: are arguments used
> twice (side-effect), are arguments unused (typechecking dager), are
> arguments cast (type-loss danger), etc. Looks very hard to implement
> though :-/ Andy, what do you think?
I think whining about anything which matches
#define foo(...) bar(
would be a decent start.
grep '^[ ]*#[]*define[ ][ ]*[^(]*[(][^)]*[)][
]*[a-zA-Z]' include/asm-x86/*.h
(hey, that worked on the first attempt)
Lots of falsies tho.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 07/18] v4l: nopage
* Andrew Morton <[EMAIL PROTECTED]> wrote:
> On Wed, 05 Dec 2007 18:15:54 +1100
> [EMAIL PROTECTED] wrote:
>
> > +static int
> > +videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> > {
> > struct page *page;
> >
> > - dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
> > - vaddr,vma->vm_start,vma->vm_end);
> > - if (vaddr > vma->vm_end)
> > - return NOPAGE_SIGBUS;
> > + dprintk(3,"fault: fault @ %08lx [vma %08lx-%08lx]\n",
> > + (unsigned long)vmf->virtual_address,vma->vm_start,vma->vm_end);
> > page = alloc_page(GFP_USER | __GFP_DMA32);
> > if (!page)
> > - return NOPAGE_OOM;
> > + return VM_FAULT_OOM;
> > clear_user_page(page_address(page), vaddr, page);
>
> This didn't compile on sparc64 because `vaddr' is undefined.
>
>
> Let us see why:
>
> #define clear_user_page(page, vaddr, pg) clear_page(page)
> #define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
>
> #define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
> alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
>
> root cause: lack of argument checking on x86 due to stupid macros.
>
>
> Could someone *please* start a little project of extirpating this
> utter brain damage? Convert those macros to typechecked static
> inlines on x86 (at least) so this sort of thing (which happens again
> and again and again) is lessened?
i wanted to write a reply to suggest a checkpatch policy for this. When
i noticed this sentence at the end of your mail:
> macros are such miserable things. I wonder if we could get checkpatch
> to help out here?
/me too :-)
any policy that gets into checkpatch.pl's default output is a policy for
arch/x86 patch merging. (as long as it's not a false positive) And
because we do all these unifications the effects of checkpatch.pl
permeate basically every aspect of arch/x86.
one approach would be to make new macros in include/* a no-no. That
would hurt a few of the legitimate uses though, so maybe a useful
refinement would be to check the structure of macros: are arguments used
twice (side-effect), are arguments unused (typechecking dager), are
arguments cast (type-loss danger), etc. Looks very hard to implement
though :-/ Andy, what do you think?
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 07/18] v4l: nopage
* Andrew Morton [EMAIL PROTECTED] wrote:
On Wed, 05 Dec 2007 18:15:54 +1100
[EMAIL PROTECTED] wrote:
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct page *page;
- dprintk(3,nopage: fault @ %08lx [vma %08lx-%08lx]\n,
- vaddr,vma-vm_start,vma-vm_end);
- if (vaddr vma-vm_end)
- return NOPAGE_SIGBUS;
+ dprintk(3,fault: fault @ %08lx [vma %08lx-%08lx]\n,
+ (unsigned long)vmf-virtual_address,vma-vm_start,vma-vm_end);
page = alloc_page(GFP_USER | __GFP_DMA32);
if (!page)
- return NOPAGE_OOM;
+ return VM_FAULT_OOM;
clear_user_page(page_address(page), vaddr, page);
This didn't compile on sparc64 because `vaddr' is undefined.
Let us see why:
#define clear_user_page(page, vaddr, pg) clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
root cause: lack of argument checking on x86 due to stupid macros.
Could someone *please* start a little project of extirpating this
utter brain damage? Convert those macros to typechecked static
inlines on x86 (at least) so this sort of thing (which happens again
and again and again) is lessened?
i wanted to write a reply to suggest a checkpatch policy for this. When
i noticed this sentence at the end of your mail:
macros are such miserable things. I wonder if we could get checkpatch
to help out here?
/me too :-)
any policy that gets into checkpatch.pl's default output is a policy for
arch/x86 patch merging. (as long as it's not a false positive) And
because we do all these unifications the effects of checkpatch.pl
permeate basically every aspect of arch/x86.
one approach would be to make new macros in include/* a no-no. That
would hurt a few of the legitimate uses though, so maybe a useful
refinement would be to check the structure of macros: are arguments used
twice (side-effect), are arguments unused (typechecking dager), are
arguments cast (type-loss danger), etc. Looks very hard to implement
though :-/ Andy, what do you think?
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 07/18] v4l: nopage
On Sat, 8 Dec 2007 10:15:08 +0100 Ingo Molnar [EMAIL PROTECTED] wrote:
* Andrew Morton [EMAIL PROTECTED] wrote:
On Wed, 05 Dec 2007 18:15:54 +1100
[EMAIL PROTECTED] wrote:
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct page *page;
- dprintk(3,nopage: fault @ %08lx [vma %08lx-%08lx]\n,
- vaddr,vma-vm_start,vma-vm_end);
- if (vaddr vma-vm_end)
- return NOPAGE_SIGBUS;
+ dprintk(3,fault: fault @ %08lx [vma %08lx-%08lx]\n,
+ (unsigned long)vmf-virtual_address,vma-vm_start,vma-vm_end);
page = alloc_page(GFP_USER | __GFP_DMA32);
if (!page)
- return NOPAGE_OOM;
+ return VM_FAULT_OOM;
clear_user_page(page_address(page), vaddr, page);
This didn't compile on sparc64 because `vaddr' is undefined.
Let us see why:
#define clear_user_page(page, vaddr, pg)clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
root cause: lack of argument checking on x86 due to stupid macros.
Could someone *please* start a little project of extirpating this
utter brain damage? Convert those macros to typechecked static
inlines on x86 (at least) so this sort of thing (which happens again
and again and again) is lessened?
We should fix existing stuff, like this.
i wanted to write a reply to suggest a checkpatch policy for this. When
i noticed this sentence at the end of your mail:
macros are such miserable things. I wonder if we could get checkpatch
to help out here?
/me too :-)
any policy that gets into checkpatch.pl's default output is a policy for
arch/x86 patch merging. (as long as it's not a false positive) And
because we do all these unifications the effects of checkpatch.pl
permeate basically every aspect of arch/x86.
one approach would be to make new macros in include/* a no-no. That
would hurt a few of the legitimate uses though, so maybe a useful
refinement would be to check the structure of macros: are arguments used
twice (side-effect), are arguments unused (typechecking dager), are
arguments cast (type-loss danger), etc. Looks very hard to implement
though :-/ Andy, what do you think?
I think whining about anything which matches
#define foo(...) bar(
would be a decent start.
grep '^[ ]*#[]*define[ ][ ]*[^(]*[(][^)]*[)][
]*[a-zA-Z]' include/asm-x86/*.h
(hey, that worked on the first attempt)
Lots of falsies tho.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 07/18] v4l: nopage
On Wed, 05 Dec 2007 18:15:54 +1100
[EMAIL PROTECTED] wrote:
> +static int
> +videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> {
> struct page *page;
>
> - dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
> - vaddr,vma->vm_start,vma->vm_end);
> - if (vaddr > vma->vm_end)
> - return NOPAGE_SIGBUS;
> + dprintk(3,"fault: fault @ %08lx [vma %08lx-%08lx]\n",
> + (unsigned long)vmf->virtual_address,vma->vm_start,vma->vm_end);
> page = alloc_page(GFP_USER | __GFP_DMA32);
> if (!page)
> - return NOPAGE_OOM;
> + return VM_FAULT_OOM;
> clear_user_page(page_address(page), vaddr, page);
This didn't compile on sparc64 because `vaddr' is undefined.
Let us see why:
#define clear_user_page(page, vaddr, pg)clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
root cause: lack of argument checking on x86 due to stupid macros.
Could someone *please* start a little project of extirpating this utter
brain damage? Convert those macros to typechecked static inlines on x86
(at least) so this sort of thing (which happens again and again and again)
is lessened?
macros are such miserable things. I wonder if we could get checkpatch to
help out here?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 07/18] v4l: nopage
On Wed, 05 Dec 2007 18:15:54 +1100
[EMAIL PROTECTED] wrote:
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct page *page;
- dprintk(3,nopage: fault @ %08lx [vma %08lx-%08lx]\n,
- vaddr,vma-vm_start,vma-vm_end);
- if (vaddr vma-vm_end)
- return NOPAGE_SIGBUS;
+ dprintk(3,fault: fault @ %08lx [vma %08lx-%08lx]\n,
+ (unsigned long)vmf-virtual_address,vma-vm_start,vma-vm_end);
page = alloc_page(GFP_USER | __GFP_DMA32);
if (!page)
- return NOPAGE_OOM;
+ return VM_FAULT_OOM;
clear_user_page(page_address(page), vaddr, page);
This didn't compile on sparc64 because `vaddr' is undefined.
Let us see why:
#define clear_user_page(page, vaddr, pg)clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
root cause: lack of argument checking on x86 due to stupid macros.
Could someone *please* start a little project of extirpating this utter
brain damage? Convert those macros to typechecked static inlines on x86
(at least) so this sort of thing (which happens again and again and again)
is lessened?
macros are such miserable things. I wonder if we could get checkpatch to
help out here?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 07/18] v4l: nopage
Convert v4l from nopage to fault.
Remove redundant vma range checks.
Signed-off-by: Nick Piggin <[EMAIL PROTECTED]>
Cc: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Cc: linux-kernel@vger.kernel.org
---
drivers/media/video/videobuf-dma-sg.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
Index: linux-2.6/drivers/media/video/videobuf-dma-sg.c
===
--- linux-2.6.orig/drivers/media/video/videobuf-dma-sg.c
+++ linux-2.6/drivers/media/video/videobuf-dma-sg.c
@@ -385,30 +385,26 @@ videobuf_vm_close(struct vm_area_struct
* now ...). Bounce buffers don't work very well for the data rates
* video capture has.
*/
-static struct page*
-videobuf_vm_nopage(struct vm_area_struct *vma, unsigned long vaddr,
- int *type)
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct page *page;
- dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
- vaddr,vma->vm_start,vma->vm_end);
- if (vaddr > vma->vm_end)
- return NOPAGE_SIGBUS;
+ dprintk(3,"fault: fault @ %08lx [vma %08lx-%08lx]\n",
+ (unsigned long)vmf->virtual_address,vma->vm_start,vma->vm_end);
page = alloc_page(GFP_USER | __GFP_DMA32);
if (!page)
- return NOPAGE_OOM;
+ return VM_FAULT_OOM;
clear_user_page(page_address(page), vaddr, page);
- if (type)
- *type = VM_FAULT_MINOR;
- return page;
+ vmf->page = page;
+ return 0;
}
static struct vm_operations_struct videobuf_vm_ops =
{
.open = videobuf_vm_open,
.close= videobuf_vm_close,
- .nopage = videobuf_vm_nopage,
+ .fault= videobuf_vm_fault,
};
/* -
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 07/18] v4l: nopage
Convert v4l from nopage to fault.
Remove redundant vma range checks.
Signed-off-by: Nick Piggin [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Cc: linux-kernel@vger.kernel.org
---
drivers/media/video/videobuf-dma-sg.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
Index: linux-2.6/drivers/media/video/videobuf-dma-sg.c
===
--- linux-2.6.orig/drivers/media/video/videobuf-dma-sg.c
+++ linux-2.6/drivers/media/video/videobuf-dma-sg.c
@@ -385,30 +385,26 @@ videobuf_vm_close(struct vm_area_struct
* now ...). Bounce buffers don't work very well for the data rates
* video capture has.
*/
-static struct page*
-videobuf_vm_nopage(struct vm_area_struct *vma, unsigned long vaddr,
- int *type)
+static int
+videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct page *page;
- dprintk(3,nopage: fault @ %08lx [vma %08lx-%08lx]\n,
- vaddr,vma-vm_start,vma-vm_end);
- if (vaddr vma-vm_end)
- return NOPAGE_SIGBUS;
+ dprintk(3,fault: fault @ %08lx [vma %08lx-%08lx]\n,
+ (unsigned long)vmf-virtual_address,vma-vm_start,vma-vm_end);
page = alloc_page(GFP_USER | __GFP_DMA32);
if (!page)
- return NOPAGE_OOM;
+ return VM_FAULT_OOM;
clear_user_page(page_address(page), vaddr, page);
- if (type)
- *type = VM_FAULT_MINOR;
- return page;
+ vmf-page = page;
+ return 0;
}
static struct vm_operations_struct videobuf_vm_ops =
{
.open = videobuf_vm_open,
.close= videobuf_vm_close,
- .nopage = videobuf_vm_nopage,
+ .fault= videobuf_vm_fault,
};
/* -
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
