[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: I/O permission bitmap patch for oskit-mach

From: Roland McGrath
Subject: Re: I/O permission bitmap patch for oskit-mach
Date: Wed, 6 Mar 2002 02:58:56 -0500 (EST)

I am just skimming the code and not thinking too deeply about it right now,
since we covered it in detail before.  Just some nits off hand:

Please add -p to your diff switches.

For ktss, don't use a weak alias, use a strong one.  You don't want some
other base_tss definition to possibly take precedence.  If there is a
multiple definition problem, that indicates a real bug we should look into.
The weak alias could mask that problem.  Do:

        static struct task_ktss ktss;
        extern struct x86_tss base_tss __attribute__ ((alias "ktss"));

I don't see a reason to zalloc the struct machine_task.  Why not just have
the struct task member be a struct instead of a pointer?  You are following
the model of pcb_t in struct thread, but I don't see much reason for that
either.  It keeps the structure opaque from more of the code, but I don't
think that really matters and is worth the indirection and zalloc overhead.

For the SMP problem, it would certainly be simple to hack the existing
check_io_fault code to notice when the task's iopb has changed, reload it,
and retry the instruction.  But I am really not concerned about this
problem cropping up in practice--certainly it won't before we finish making
SMP work at all! :)

Your copyright headers for new files are right on (for wholly new code
whose authors are assigning copyright to FSF).  New code we've put into
Mach has already been under the GPL--in gnumach the Linux drivers and their
glue code are already GPL'd, and in oskit-mach the oskit itself and also
much code in oskit/ that I derived from oskit code, is already GPL'd.  In
fact, I have been lax about copyright headers for the new files in
oskit-mach.  I just didn't want to worry about that so long before the code
was ever going to be released for general use; but that time is getting closer.

In io_bitmap_init, just use memset.  That's what it's for.

Might as well use io_port_t for fields like iopb_size.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]