Broken kunmap calls in rc4-mm1.

Broken kunmap calls in rc4-mm1.

[Nigel Cunningham]

Hi Andrew et al.

That oops in kunmap got me thinking of my recent DEBUG_HIGHMEM
additions, so I want for a walk through the -mm4 patch, and found plenty
of instances of people making the same mistake I did... using the struct
page * in the call to kunmap, rather than the virtual address.

I guess the best way to handle it is find/notify the respective authors
of patches in the tree? The problems are in:

Reiser4 (lots)
CacheFS (lots)
afs
binfmt_elf
libata_core

(I'm hoping some of the above people will see this message and save me
some effort :>)

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

You see, at just the right time, when we were still powerless, Christ
died for the ungodly.		-- Romans 5:6

Re: Broken kunmap calls in rc4-mm1.

[William Lee Irwin III]

On Thu, Nov 11, 2004 at 12:17:05PM +1100, Nigel Cunningham wrote:
> That oops in kunmap got me thinking of my recent DEBUG_HIGHMEM
> additions, so I want for a walk through the -mm4 patch, and found plenty
> of instances of people making the same mistake I did... using the struct
> page * in the call to kunmap, rather than the virtual address.
> I guess the best way to handle it is find/notify the respective authors
> of patches in the tree? The problems are in:
> Reiser4 (lots)
> CacheFS (lots)
> afs
> binfmt_elf
> libata_core
> (I'm hoping some of the above people will see this message and save me
> some effort :>)

That only applies to kunmap_atomic(); kunmap()'s argument should be a page.


-- wli

Re: Broken kunmap calls in rc4-mm1.

[Chris Wright]

* Nigel Cunningham (ncunningham@linuxmail.org) wrote:
> Hi Andrew et al.
> 
> That oops in kunmap got me thinking of my recent DEBUG_HIGHMEM
> additions, so I want for a walk through the -mm4 patch, and found plenty
> of instances of people making the same mistake I did... using the struct
> page * in the call to kunmap, rather than the virtual address.
> 
> I guess the best way to handle it is find/notify the respective authors
> of patches in the tree? The problems are in:
> 
> Reiser4 (lots)
> CacheFS (lots)
> afs
> binfmt_elf
> libata_core

I think you are confusing with kunmap_atomic?

thanks
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: Broken kunmap calls in rc4-mm1.

[Nigel Cunningham]

Hi.

On Thu, 2004-11-11 at 12:29, William Lee Irwin III wrote:
> On Thu, Nov 11, 2004 at 12:17:05PM +1100, Nigel Cunningham wrote:
> > That oops in kunmap got me thinking of my recent DEBUG_HIGHMEM
> > additions, so I want for a walk through the -mm4 patch, and found plenty
> > of instances of people making the same mistake I did... using the struct
> > page * in the call to kunmap, rather than the virtual address.
> > I guess the best way to handle it is find/notify the respective authors
> > of patches in the tree? The problems are in:
> > Reiser4 (lots)
> > CacheFS (lots)
> > afs
> > binfmt_elf
> > libata_core
> > (I'm hoping some of the above people will see this message and save me
> > some effort :>)
> 
> That only applies to kunmap_atomic(); kunmap()'s argument should be a page.

Ah. My bad. That cuts it down by quite a few. I should have stuck to
looking for kmap_atomic :>

Humble apologies to those wrongly maligned!

Remaining culprits are....

Reiser4:
- do_readpage_tail
 -reiser4_status_init
 -reiser4_status_write

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

You see, at just the right time, when we were still powerless, Christ
died for the ungodly.		-- Romans 5:6

Re: Broken kunmap calls in rc4-mm1.

[Andrew Morton]

Nigel Cunningham <ncunningham@linuxmail.org> wrote:
>
> Remaining culprits are....
> 
>  Reiser4:
>  - do_readpage_tail
>   -reiser4_status_init
>   -reiser4_status_write

obuggerit.  Look, a simple helper is to redefine kmap_atomic() and
kunmap_atomic() to work on char*'s.  This will spit warnings if someone
feeds in a page*.  Which would be a lot more useful if we didn't have all
those infernal __iomem warnings scrolling off the screen but ho hum.

I'll cook something up.

Re: Broken kunmap calls in rc4-mm1.

[Jens Axboe]

On Wed, Nov 10 2004, Andrew Morton wrote:
> Nigel Cunningham <ncunningham@linuxmail.org> wrote:
> >
> > Remaining culprits are....
> > 
> >  Reiser4:
> >  - do_readpage_tail
> >   -reiser4_status_init
> >   -reiser4_status_write
> 
> obuggerit.  Look, a simple helper is to redefine kmap_atomic() and
> kunmap_atomic() to work on char*'s.  This will spit warnings if someone
> feeds in a page*.  Which would be a lot more useful if we didn't have all
> those infernal __iomem warnings scrolling off the screen but ho hum.

I tried something like this, but got stuck on people passing a structure
in which is valid (aio_setup_ring(), for instance). We can always cast
those of course, but it's not so pretty.

-- 
Jens Axboe

Re: Broken kunmap calls in rc4-mm1.

[Andrew Morton]

Jens Axboe <axboe@suse.de> wrote:
>
> On Wed, Nov 10 2004, Andrew Morton wrote:
> > Nigel Cunningham <ncunningham@linuxmail.org> wrote:
> > >
> > > Remaining culprits are....
> > > 
> > >  Reiser4:
> > >  - do_readpage_tail
> > >   -reiser4_status_init
> > >   -reiser4_status_write
> > 
> > obuggerit.  Look, a simple helper is to redefine kmap_atomic() and
> > kunmap_atomic() to work on char*'s.  This will spit warnings if someone
> > feeds in a page*.  Which would be a lot more useful if we didn't have all
> > those infernal __iomem warnings scrolling off the screen but ho hum.
> 
> I tried something like this, but got stuck on people passing a structure
> in which is valid (aio_setup_ring(), for instance). We can always cast
> those of course, but it's not so pretty.

yup, that's what I did.  It's not too bad, apart from cachefs which has
gone kmap nutso.

Re: Broken kunmap calls in rc4-mm1.

[Pavel Machek]

Hi!

> > That oops in kunmap got me thinking of my recent DEBUG_HIGHMEM
> > additions, so I want for a walk through the -mm4 patch, and found plenty
> > of instances of people making the same mistake I did... using the struct
> > page * in the call to kunmap, rather than the virtual address.
> > I guess the best way to handle it is find/notify the respective authors
> > of patches in the tree? The problems are in:
> > Reiser4 (lots)
> > CacheFS (lots)
> > afs
> > binfmt_elf
> > libata_core
> > (I'm hoping some of the above people will see this message and save me
> > some effort :>)
> 
> That only applies to kunmap_atomic(); kunmap()'s argument should be a page.

Is it just me or is having two very similar functions
(kunmap/kunmap_atomic) pretty counter-intuitive? Perhaps kunmap should
be renamed to kunmap_page() or something?

								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!