2.4.26-rc1-pac1

2.4.26-rc1-pac1

[bero]

... can be found at

ftp://yourfavoritemirror.kernel.org/pub/linux/kernel/people/bero/2.4/2.4.26/

Changes:
- Port everything to 2.4.26-rc1
- DRI updates
- Fix up the S3 Savage DRI driver to at least compile (lacking the 
  hardware, I can't tell whether or not it works - feedback welcome)
- ACPI updates

LLaP
bero

-- 
Ark Linux - Linux for the masses
http://www.arklinux.org/

Redistribution and processing of this message is subject to
http://www.arklinux.org/terms.php

To kunmap_atomic or not to kunmap_atomic ?

[Zoltan Menyhart]

I can see a couple of functions, like

static inline struct mm_struct * ptep_to_mm(pte_t * ptep)
{
	struct page * page = kmap_atomic_to_page(ptep);
	return (struct mm_struct *) page->mapping;
}

in "rmap.?" without invoking "kunmap_atomic()".
Is it intentional?
What if for an architecture "kunmap_atomic()" is not a no-op ?
Thanks,

Zoltan Menyhart

Re: To kunmap_atomic or not to kunmap_atomic ?

[Hugh Dickins]

On Thu, 1 Apr 2004, Zoltan Menyhart wrote:
> I can see a couple of functions, like
> 
> static inline struct mm_struct * ptep_to_mm(pte_t * ptep)
> {
> 	struct page * page = kmap_atomic_to_page(ptep);
> 	return (struct mm_struct *) page->mapping;
> }
> 
> in "rmap.?" without invoking "kunmap_atomic()".
> Is it intentional?
> What if for an architecture "kunmap_atomic()" is not a no-op ?

Amusing misunderstanding.  Take a look at kmap_atomic_to_page
in arch/i386/mm/highmem.c: it doesn't _do_ a kmap_atomic, it
translates the virtual address already supplied by kmap_atomic
to the address of the struct page of the physical page backing
that virtual address.  So, in the case of try_to_unmap_one, it
operates on the virtual address supplied by rmap_ptep_map
(which does do a kmap_atomic), and at the end there's an
rmap_ptep_unmap (which does the rmap_ptep_unmap).

Hugh

Re: To kunmap_atomic or not to kunmap_atomic ?

[Zoltan Menyhart]

Hugh Dickins wrote:
> 
> On Thu, 1 Apr 2004, Zoltan Menyhart wrote:
> > I can see a couple of functions, like
> >
> > static inline struct mm_struct * ptep_to_mm(pte_t * ptep)
> > {
> >       struct page * page = kmap_atomic_to_page(ptep);
> >       return (struct mm_struct *) page->mapping;
> > }
> >
> > in "rmap.?" without invoking "kunmap_atomic()".
> > Is it intentional?
> > What if for an architecture "kunmap_atomic()" is not a no-op ?
> 
> Amusing misunderstanding.  Take a look at kmap_atomic_to_page
> in arch/i386/mm/highmem.c: it doesn't _do_ a kmap_atomic, it
> translates the virtual address already supplied by kmap_atomic
> to the address of the struct page of the physical page backing
> that virtual address.  So, in the case of try_to_unmap_one, it
> operates on the virtual address supplied by rmap_ptep_map
> (which does do a kmap_atomic), and at the end there's an
> rmap_ptep_unmap (which does the rmap_ptep_unmap).
> 
> Hugh

As you have *map* - *unmap* macros / functions, they give a _hint_
that the original intention of the author was to allow some
architectures - which do need some resources to map things -
manage these resources. See:

static inline void *kmap(struct page *page)
#define kunmap(page)

#define kmap_atomic(page, idx)
#define kunmap_atomic(addr, idx)
#define kmap_atomic_to_page(ptr)

I think we cannot guarantee that we will never ever need to
unmap things. As it is required to use kmap - kunmap in pair,
it is quite logic to use kmap_atomic* in pair with kunmap_atomic.

I think it is a bad programming style to abuse the fact that
some macros are no-ops for the most popular architectures.

I think we should have some global counters in DEBUG mode which
are incremented on each call to *map* and decremented on each
*unpap* call, and we can detect, ooops, it leaks...

Regards,

Zoltán Menyhárt

Re: To kunmap_atomic or not to kunmap_atomic ?

[Hugh Dickins]

On Fri, 2 Apr 2004, Zoltan Menyhart wrote:
> > 
> > Amusing misunderstanding.  Take a look at kmap_atomic_to_page
> > in arch/i386/mm/highmem.c: it doesn't _do_ a kmap_atomic, it
> > translates the virtual address already supplied by kmap_atomic
> > to the address of the struct page of the physical page backing
> > that virtual address.  So, in the case of try_to_unmap_one, it
> > operates on the virtual address supplied by rmap_ptep_map
> > (which does do a kmap_atomic), and at the end there's an
> > rmap_ptep_unmap (which does the rmap_ptep_unmap).
>...
> 
> I think we cannot guarantee that we will never ever need to
> unmap things. As it is required to use kmap - kunmap in pair,
> it is quite logic to use kmap_atomic* in pair with kunmap_atomic.

Agreed.

> I think it is a bad programming style to abuse the fact that
> some macros are no-ops for the most popular architectures.

Agreed.

> I think we should have some global counters in DEBUG mode which
> are incremented on each call to *map* and decremented on each
> *unpap* call, and we can detect, ooops, it leaks...

If you choose CONFIG_DEBUG_HIGHMEM, kmap_atomic does check that
kunmap_atomic was done last time, no need for additional counter.

Sorry, I've not made it clear.

kmap_atomic_to_page does not map anything, and doesn't need a
corresponding unmap operation.  It expects that you already did
a kmap_atomic (and that you will later do a kunmap_atomic), and
translates from the address returned by kmap_atomic to the address
of the relevant struct page.  You can certainly argue that it's
misleadingly named, but no better name springs to my mind.

Hugh