William Lee Irwin III wrote:
On Fri, Jun 24, 2005 at 09:21:31AM +1000, Nick Piggin wrote:
This patch doesn't care how it works, that would be for a later patch.
The general gist of all this is that the patch doesn't cover anywhere
near enough ground, and so the above illustrates that the usage
in/around bootmem is among the easiest of usages to remove. I have
implemented what I'm talking about on several independent occasions.
I don't think you understand what ground the patch covers.
It removes PageReserved() queries from core code, leaving
all PG_reserved flags and other uses of it intact so it
doesn't cause mass breakage.
Things can then be looked at and fixed up properly at a slower
pace while the patch is in -mm, for example.
William Lee Irwin III wrote:
This is called in the interior of a loop, which may be beneficial to
terminate if this intended semantic is to be enforced. Furthermore, no
error is propagated to the caller, which is not the desired effect in
the stated error reporting scheme. So the code is inconsistent with
explicitly stated intention.
On Fri, Jun 24, 2005 at 09:21:31AM +1000, Nick Piggin wrote:
No, the error reporting scheme says it doesn't handle any error,
that is all. What we have here in terms of behaviour is exactly
what used to happen, that is - do something saneish on error.
Changing behaviour would be outside the scope of this patch, but
be my guest.
Some places BUG(), some back out with an error return, others blithely
proceed. This kind of inconsistency will broadly confuse callers of
the API's.
I don't think any places BUG(), but regardless, this patch doesn't
deal with changing behaviour, just reporting of the problem.
William Lee Irwin III wrote:
You are going on about the fact that install_page() can't be used on
memory outside mem_map[] as it requires a page structure, and can't be
used on reserved pages because page_add_file_rmap() will BUG. This case
is not being discussed.
On Fri, Jun 24, 2005 at 09:21:31AM +1000, Nick Piggin wrote:
And that it isn't allowed to touch struct page of physical pages
in a VM_RESERVED region.
non sequitur
Huh? That is why it is broken. Previously, PageReserved pages
*were not* accounted with the normal rmap and friends in core
code, so you can't just do it in here and hope it works.
William Lee Irwin III wrote:
The issue at stake is inserting normal pages into a VM_RESERVED vma.
These will arise as e.g. kernel-allocated buffers managed by normal
reference counting. remap_pfn_range() can't do it; it refuses to
operate on "valid memory". install_page() now won't do it; it refuses
to touch a VM_RESERVED vma. So this creates a giant semantic hole,
and potentially breaks working code (i.e. if you were going to do
this you would need not only a replacement but also a sweep to adjust
all the drivers doing it or prove their nonexistence).
On Fri, Jun 24, 2005 at 09:21:31AM +1000, Nick Piggin wrote:
I think you'll find that remap_pfn_range will be happy to operate
on valid memory, and that any driver trying to use install_page
on VM_RESERVED probably needs fixing anyway.
install_page() of a !PageReserved() page in a VM_RESERVED vma is
neither broken now nor does it merit going BUG().
Hugh says it is broken, so you can ask him the details.
/dev/mem was disallowed from mapping ordinary kernel memory for a
reason (though I disagree with it), so the removal of the
pfn_valid()/PageReserved() checks can't be blithely done like in
your patch. Other arrangements must be made.
William Lee Irwin III wrote:
Unfortunately for this scheme, it's very much a case of putting the
cart before the horse. PG_reserved is toggled at random in this driver
after this change, to no useful effect (debugging or otherwise). And
this really goes for the whole affair. Diddling the core first is just
going to create bugs. Converting the users first is the way these
things need to be done. When complete, nothing needs the core flags
twiddling anymore and you just nuke the flag twiddling from the core.
On Fri, Jun 24, 2005 at 09:21:31AM +1000, Nick Piggin wrote:
I'm sorry, I don't see how 'diddling' the core will create bugs.
This is a fine way to do it, and "converting" users first (whatever
that means)
[cut text included in full in the following quote block]
This is going way too far. Someone please deal with this.
No, just tell me how it might magically create bugs in drivers
that didn't exist in the first place?
On Fri, Jun 24, 2005 at 09:21:31AM +1000, Nick Piggin wrote:
[continued]
This is a fine way to do it, and "converting" users first (whatever
that means) is not possible because VM_RESERVED handling in core
code is not up to the task of replacing PageReserved without this
patch.
You aren't replacing any of the PG_reserved usage with VM_RESERVED
in any of these patches. The primary motive of all this is AFAICT
little more than getting the PG_reserved check out of put_page(),
drivers and arches be damned.
Excuse me? drivers work, arches work.
And how might you "fix" them first, without the necessary core
code in place?
Send instant messages to your online friends http://au.messenger.yahoo.com
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
