[git pull] AGP fixes for 2.6.23-rc1

[git pull] AGP fixes for 2.6.23-rc1

[Dave Airlie]

Hi Linus,

Please pull the 'agp-patches' branch from
git://master.kernel.org/pub/scm/linux/kernel/git/airlied/agp-2.6.git agp-patches

It contains one very important fix from Nick for AGP hangs with the 
nopage/pfn/flush stuff.. it also contains some minor docs changes and one 
coverity fix.

Dave.

  Documentation/kernel-parameters.txt |    7 +++++++
  drivers/char/agp/Kconfig            |    2 +-
  drivers/char/agp/ati-agp.c          |    9 ++-------
  drivers/char/agp/generic.c          |    2 --
  drivers/char/agp/intel-agp.c        |   19 ++++++++++++-------
  drivers/char/agp/sgi-agp.c          |    1 -
  6 files changed, 22 insertions(+), 18 deletions(-)

commit f191f144079b0083c6fa7d01a4acbd7263fb5032
Author: Alan Hourihane <alanh@fairlite.demon.co.uk>
Date:   Fri Jul 27 10:56:43 2007 +1000

     agp: AMD AGP is used on UP1100 & UP1500 alpha boxen

     Signed-off-by: Dave Airlie <airlied@redhat.com>

commit dde4787642ee3cb85aef80bdade04b6f8ddc3df8
Author: Zhenyu Wang <zhenyu.z.wang@intel.com>
Date:   Thu Jul 26 09:18:09 2007 +0800

     intel_agp: really fix 945/965GME

     Fix some missing places to check with device id info, which
     should probe the device gart correctly.

     Signed-off-by: Wang Zhenyu <zhenyu.z.wang@intel.com>
     Signed-off-by: Dave Airlie <airlied@redhat.com>

commit a51b34593f691a0837d752a1394dcee19483c607
Author: Nick Piggin <npiggin@suse.de>
Date:   Wed Jul 25 13:19:22 2007 +0200

     agp: don't lock pages

     AGP should not need to lock pages. They are not protecting any race
     because there is no lock_page calls, only SetPageLocked.

     This is causing hangs with d00806b183152af6d24f46f0c33f14162ca1262a.

     Signed-off-by: Nick Piggin <npiggin@suse.de>
     Signed-off-by: Dave Airlie <airlied@redhat.com>

commit c99c108ac362f5cc37f79fad7e9897bd9d033bcc
Author: Chuck Ebbert <cebbert@redhat.com>
Date:   Fri Jul 27 10:46:20 2007 +1000

     AGP: document boot options

     Add documentation for AGP boot options.

     Signed-off-by: Chuck Ebbert <cebbert@redhat.com>
     Signed-off-by: Dave Airlie <airlied@redhat.com>

commit 190644e180794208bc638179f4d5940fe419bf9c
Author: Jesper Juhl <jesper.juhl@gmail.com>
Date:   Sat Jul 21 17:39:11 2007 +0200

     Fix "use after free" / "double free" bug in ati_create_gatt_pages / ati_free_gatt_pages

     Hi,

     Coverity spotted a "use after free" bug in
     drivers/char/agp/ati-agp.c::ati_create_gatt_pages().

     The same one that was in
       drivers/char/agp/amd-k7-agp.c::amd_create_gatt_pages()

     The problem is this:
            If "entry = kzalloc(sizeof(struct ati_page_map), GFP_KERNEL);"
     fails, then there's a loop in the function to free all entries
     allocated so far and break out of the allocation loop. That in itself
     is pretty sane, but then the (now freed) 'tables' is assigned to
     ati_generic_private.gatt_pages and 'retval' is set to -ENOMEM which
     causes ati_free_gatt_pages(); to be called at the end of the function.
     The problem with this is that ati_free_gatt_pages() will then loop
     'ati_generic_private.num_tables' times and try to free each entry in
     tables[] - this is bad since tables has already been freed and
     furthermore it will call kfree(tables) at the end - a double free.

     This patch removes the freeing loop in ati_create_gatt_pages() and
     instead relies entirely on the call to ati_free_gatt_pages() to free
     everything we allocated in case of an error. It also sets
     ati_generic_private.num_tables to the actual number of entries
     allocated instead of just using the value passed in from the caller -
     this ensures that ati_free_gatt_pages() will only attempt to free
     stuff that was actually allocated.

     Note: I'm in no way intimate with this code and I have no way to
     actually test this patch (besides compile test it), so while I've
     tried to be careful in reading the code and make sure the patch
     does the right thing an ACK from someone who actually knows the
     code in-depth would be very much appreciated.

     Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
     Signed-off-by: Dave Airlie <airlied@linux.ie>

Re: [git pull] AGP fixes for 2.6.23-rc1

[Dave Airlie]

On Fri, 27 Jul 2007, Dave Airlie wrote:

>
> Hi Linus,
>
> Please pull the 'agp-patches' branch from
> git://master.kernel.org/pub/scm/linux/kernel/git/airlied/agp-2.6.git agp-patches

and of course I find the old version of my script..

that should be
ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/agp-2.6.git agp-patches

Dave.

>
> It contains one very important fix from Nick for AGP hangs with the 
> nopage/pfn/flush stuff.. it also contains some minor docs changes and one 
> coverity fix.
>
> Dave.
>
> Documentation/kernel-parameters.txt |    7 +++++++
> drivers/char/agp/Kconfig            |    2 +-
> drivers/char/agp/ati-agp.c          |    9 ++-------
> drivers/char/agp/generic.c          |    2 --
> drivers/char/agp/intel-agp.c        |   19 ++++++++++++-------
> drivers/char/agp/sgi-agp.c          |    1 -
> 6 files changed, 22 insertions(+), 18 deletions(-)
>
> commit f191f144079b0083c6fa7d01a4acbd7263fb5032
> Author: Alan Hourihane <alanh@fairlite.demon.co.uk>
> Date:   Fri Jul 27 10:56:43 2007 +1000
>
>    agp: AMD AGP is used on UP1100 & UP1500 alpha boxen
>
>    Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> commit dde4787642ee3cb85aef80bdade04b6f8ddc3df8
> Author: Zhenyu Wang <zhenyu.z.wang@intel.com>
> Date:   Thu Jul 26 09:18:09 2007 +0800
>
>    intel_agp: really fix 945/965GME
>
>    Fix some missing places to check with device id info, which
>    should probe the device gart correctly.
>
>    Signed-off-by: Wang Zhenyu <zhenyu.z.wang@intel.com>
>    Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> commit a51b34593f691a0837d752a1394dcee19483c607
> Author: Nick Piggin <npiggin@suse.de>
> Date:   Wed Jul 25 13:19:22 2007 +0200
>
>    agp: don't lock pages
>
>    AGP should not need to lock pages. They are not protecting any race
>    because there is no lock_page calls, only SetPageLocked.
>
>    This is causing hangs with d00806b183152af6d24f46f0c33f14162ca1262a.
>
>    Signed-off-by: Nick Piggin <npiggin@suse.de>
>    Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> commit c99c108ac362f5cc37f79fad7e9897bd9d033bcc
> Author: Chuck Ebbert <cebbert@redhat.com>
> Date:   Fri Jul 27 10:46:20 2007 +1000
>
>    AGP: document boot options
>
>    Add documentation for AGP boot options.
>
>    Signed-off-by: Chuck Ebbert <cebbert@redhat.com>
>    Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> commit 190644e180794208bc638179f4d5940fe419bf9c
> Author: Jesper Juhl <jesper.juhl@gmail.com>
> Date:   Sat Jul 21 17:39:11 2007 +0200
>
>    Fix "use after free" / "double free" bug in ati_create_gatt_pages / 
> ati_free_gatt_pages
>
>    Hi,
>
>    Coverity spotted a "use after free" bug in
>    drivers/char/agp/ati-agp.c::ati_create_gatt_pages().
>
>    The same one that was in
>      drivers/char/agp/amd-k7-agp.c::amd_create_gatt_pages()
>
>    The problem is this:
>           If "entry = kzalloc(sizeof(struct ati_page_map), GFP_KERNEL);"
>    fails, then there's a loop in the function to free all entries
>    allocated so far and break out of the allocation loop. That in itself
>    is pretty sane, but then the (now freed) 'tables' is assigned to
>    ati_generic_private.gatt_pages and 'retval' is set to -ENOMEM which
>    causes ati_free_gatt_pages(); to be called at the end of the function.
>    The problem with this is that ati_free_gatt_pages() will then loop
>    'ati_generic_private.num_tables' times and try to free each entry in
>    tables[] - this is bad since tables has already been freed and
>    furthermore it will call kfree(tables) at the end - a double free.
>
>    This patch removes the freeing loop in ati_create_gatt_pages() and
>    instead relies entirely on the call to ati_free_gatt_pages() to free
>    everything we allocated in case of an error. It also sets
>    ati_generic_private.num_tables to the actual number of entries
>    allocated instead of just using the value passed in from the caller -
>    this ensures that ati_free_gatt_pages() will only attempt to free
>    stuff that was actually allocated.
>
>    Note: I'm in no way intimate with this code and I have no way to
>    actually test this patch (besides compile test it), so while I've
>    tried to be careful in reading the code and make sure the patch
>    does the right thing an ACK from someone who actually knows the
>    code in-depth would be very much appreciated.
>
>    Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
>    Signed-off-by: Dave Airlie <airlied@linux.ie>
>
>

-- 
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
Linux kernel - DRI, VAX / pam_smb / ILUG

Re: [git pull] AGP fixes for 2.6.23-rc1

[Jeff Garzik]

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

Dave Airlie wrote:
> 
> Hi Linus,
> 
> Please pull the 'agp-patches' branch from
> git://master.kernel.org/pub/scm/linux/kernel/git/airlied/agp-2.6.git 
> agp-patches
> 
> It contains one very important fix from Nick for AGP hangs with the 
> nopage/pfn/flush stuff.. it also contains some minor docs changes and 
> one coverity fix.
> 
> Dave.
> 
>  Documentation/kernel-parameters.txt |    7 +++++++
>  drivers/char/agp/Kconfig            |    2 +-
>  drivers/char/agp/ati-agp.c          |    9 ++-------
>  drivers/char/agp/generic.c          |    2 --
>  drivers/char/agp/intel-agp.c        |   19 ++++++++++++-------
>  drivers/char/agp/sgi-agp.c          |    1 -
>  6 files changed, 22 insertions(+), 18 deletions(-)

It would really be nice to actually see the patches that go along with 
this.  Otherwise you are not giving outside reviewers any chance at all 
to evaluate these fixes.

Or were they posted on LKML, and I missed them?

Attached is the shell script I use to submit stuff to Linus, in case 
that helps any.

	Jeff




[-- Attachment #2: mkmsg.sh --]
[-- Type: application/x-shellscript, Size: 546 bytes --]

Re: [git pull] AGP fixes for 2.6.23-rc1

[Dave Airlie]

>
> It would really be nice to actually see the patches that go along with this. 
> Otherwise you are not giving outside reviewers any chance at all to evaluate 
> these fixes.
>
> Or were they posted on LKML, and I missed them?

In this case I think I picked up all of these patches from LKML by 
searching for agp in my gmail a/c, maybe one of them didn't go via lkml 
(intel pci ids)

Thanks for the script,
Dave.

-- 
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
Linux kernel - DRI, VAX / pam_smb / ILUG