[haiku-commits] Re: haiku: hrev53378 - in src: system/kernel/vm kits/media

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 16 Aug 2019 00:31:43 -0400

On Thu, Aug 15, 2019 at 10:15 AM Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> wrote:

Am 15/08/2019 um 02:13 schrieb waddlesplash:
72b37d9ffc7e: kernel: Turn the clone-area-attempt panic into a dprintf.
[...]
      if (!kernel && sourceAddressSpace != targetAddressSpace
              && (sourceArea->protection & B_CLONEABLE_AREA) == 0) {
-             // kernel areas must not be cloned in userland, unless 
explicitly
-             // declared user-cloneable upon construction
  #if KDEBUG
-             panic("attempting to clone area \"%s\" (%" B_PRId32 ")!",
-                     sourceArea->name, sourceID);
+             Team* team = thread_get_current_thread()->team;
+             dprintf("team \"%s\" (%" B_PRId32 ") attempted to clone area 
\"%s\" (%"
+                     B_PRId32 ")!\n", team->Name(), team->id, 
sourceArea->name, sourceID);

That's not really such a good idea IMO.
First of all, the comment was not updated when the semantics of this
check changed.

Yes, that was my fault.


Then, it may make sense to only dprintf() for userland areas for now (if
only because this would break compatibility otherwise), but for kernel
areas, I would insist on keeping the panic.

Well, when we are called by the kernel, you can see we permit cloning
any area (and the USB stack takes advantage of this.) But otherwise
this is about userland cloning kernel areas; and those changes were
merged around this time last year. So this code has been in place for
a full year now, and I think it's safe to say we have caught all the
in-tree offenders at least.

I made it a panic to begin with because the most likely users were
stuff like graphics drivers, where if the areas were not clonable, you
would end up with a black screen (or worse, a cryptic kernel panic or
app server crash elsewhere.) Now that we have smoothed all those
issues out, and the first round of userland cloning is done, I think
it's "safe" to turn this into a dprintf. That way, things which are
clearly and obviously broken will still get noticed and we can fix
them; and people writing drivers, etc. can have a way to notice these
issues, too. Perhaps I should remove the KDEBUG guards now then?

-waddlesplash

Other related posts: