The old TTY driver had one set of indexes for both master
and slave TTYs, and it used those indexes interchangeably.
We thus need to check both lists here also.
Fixes SSH passphrase prompts not working.
This new driver uses the "generic" TTY layer, unlike the old driver
which had its own implementation (which the generic module was derived
from, originally.)
The remaining bits of support for controlling TTYs is added to the kernel &
generic layer at the same time, which should allow for serial interfaces
to be controlling terminals now, as well.
Tested with bash, nano, vim; all seems to still be working as expected.
This was used by the BeOS port of ncurses, as well as our own ncurses
port until not too long before the alpha1 days, so we should keep it
around for compatibility.
Implementation copied directly from the "legacy" TTY driver.
It doesn't make much sense to have separate window sizes, termios, etc.
settings for the two halves of a TTY. Moreover, having separate settings
which can get out of sync breaks applications, e.g. double-printing
in shells.
The original TTY driver has unified settings. It seems likely that the
settings were separated as part of the locking simplification, however
the lock separation was reverted a while back, anyway.
I am not sure how this path could be hit besides having
O_APPEND set on a socket, which appears to be possible,
though I don't know what purpose that would serve.
Tested by adding these two lines between the sleep() and close()
in the in-tree tcp_connection_test:
fcntl(fd, F_SETFL, O_APPEND);
write(fd, "Hello", 5);
Before this commit, the above lines cause a KDL.
May fix#18133, but I don't presently have access to the
reproduction setup described in that ticket.
Until the introduction of the nvme_disk driver, these classes were
mostly only used directly by the IO scheduler, and then a few direct
usages of IOOperation itself in the individual disk drivers; so
API confusions were easily missed.
When writing the nvme_disk driver's IORequest support, however, it
became readily apparent that there were some pretty bad confusions
around transferred-bytes accounting in IOOperation. This commit
attempts to resolve all of those.
There are two basic changes here:
1. Move transferred-bytes accounting into IOOperation::SetStatus.
The "TransferredBytes" field of IOOperation is against the *original*
range, not the actual operation's range (which will be wider, due to
bouncing, etc.), and furthermore only applies to the actual content
of the request (and not e.g. to a read half of a bounced write.)
These two facts meant that determining what value to pass to
SetTransferredBytes was not trivial, and was easy to get wrong.
I recall messing that up when working on nvme_disk multiple times
before reading the API carefully.
2. Do not pass redundant values to IORequest::OperationFinished.
All of the values here can be derived (albeit indirectly) from the
IOOperation, and all consumers of this API basically did just that.
Rather than make them do it, make the IORequest take care of
computing all of those values itself.
Change-Id: Ic9ae29e1100319e5b7647647c4db7e5aad4d125e
This substitutes for the already-existing behavior of scsi_cd, scsi_disk,
and virtio_block, so we can delete their hooks and let them use
these new generic fallbacks.
Some other drivers perform clamping, and so using these fallbacks would
constitute a behavioral change.
Change-Id: I9a2e503f2e03abc276bdfc02d1cff1565a9742e9
* Move geometry fetching to open(); it was not done
in io() and was duplicated in multiple places.
* Clean up clamping and bounds checking.
* Use IORequest::TransferredBytes().
Change-Id: I4157e516098dc0362c1478abd21a545c1f235cd7
The user address space has not included NULL for a while, so this
has actually been broken for years, and nobody noticed. I guess
XSI message queues are not very well used?
Fixes the in-tree "xsi_msg_queue_test1".
This removes a lot of custom logic for managing waiting threads,
which was not even correct in all cases (and the code actually
acknowledged this with a big TODO about it, which weinhold
added all the way back in 2008!)
THREAD_BLOCK_TYPE_OTHER implies the "object" pointer in the
wait information is a string. But sometimes we want to pass
through objects which are not strings, for inspection in KDL.
We need to store the isGlobal value, so that the destructor
can take care of unsetting the glibc locale properly.
It seems this has been broken since d338200e2b.
Fixes#18344 and probably #18336.
We cannot actually initiate scans, but we can at least detect when
one is in progress.
This fixes "ifconfig ... scan" on OpenBSD devices returning errors
instead of scan results ("list" always worked.)
The condition variable that the load_image'ing thread is waiting on
is also owned by that same thread, so as soon as it wakes up, it
will soon return, thus destroying it. Under high load or other unlucky
scheduling conditions, it seems this could occur before the other thread
had even returned from the condition variable's NotifyAll.
Since team->loading_info is protected by the team lock, simply
acquire the team lock once more after being awoken and returning,
to synchronize and prevent this race.
Should fix#18352.
See #18351 for details on the specifications.
This is the same thing NetBSD does. BeOS R5 defined these values
differently than we did even before this commit, and it does not
seem to have caused problems then, so this should be fine.
While technically an ABI break, in practice these values are not
always differentiated on other platforms, and it appears musl's
code triggers divide-by-zero exceptions on purpose before it
returns this value, anyway.
Fixes#18351.
* Make some code common in Insert.
* Remove unneeded logic from InsertAfter and just call the base Insert.
InsertBefore does this, already.
* Synchronize Insert code and APIs in the fs_shell DoublyLinkedList.
Removing it outright would cause implicit conversions and then the other
variant being called, which would create invalid lists. So make it private
so that any attempts to use it will create errors at compile-time.
The Insert(before, element) function has been marked deprecated since
2010, but still had many usage and kept accumulating more. It's long
past time we got rid of all them and actually deprecated the function
itself.
Insert(before, element) just calls InsertBefore, so no functional change.
These caught the problem fixed in prior commits, and would have saved
me an awful lot of debugging time had they existed from the start.
They cannot catch all double-list-insertions, of course, but it's
relatively cheap to add these, and more than nothing.
* Create an internal variant which accepts a "bool locked" parameter.
Use this from callout_reset instead of invoking it before locking,
and then relocking afterwards. Eliminates some possible (though,
so far as I know, benign) races.
* mtx_assert always, even if the callout is not active. (Requirement
notated in the comment.)
* In callout_reset, do not invoke callout_stop at all unless we are
cancelling; in cases of mere reschedules, simply change c_due
and notify the callout thread.
* While at it, use list_init_etc in init_callout; no-op change,
but keeps things clean.
This should fix#18338 (specifically the change to never invoke
callout_stop when merely rescheduling, not cancelling.)
Here is the scenario in which this matters:
1. A callout comes due, it is removed from the list, and c_due is set
to 0. We then wind up in the mtx_lock here inside invoke_callout;
while some other thread holds the lock.
2. The other thread, holding the lock, decides to *re*schedule the callout
for a later time than the present. callout_reset sees the callout has
a c_due of 0, thus indicating it is not in the sTimers list, adds it,
and sets a (future) c_due.
3. The other thread unlocks c_mtx, thus the callout thread acquires it
and proceeds.
Under the code I wrote in hrev56875, before this commit, the following
would occur (at least with the ipro1000 driver on some hardware):
4. c_due would be set to -1, thus signaling the callout was no longer
scheduled, and thus not in the sTimers list (despite being in it!).
The callout's own method would decide to reschedule itself, and
invoke callout_reset as such.
5. callout_reset would, seeing c_due of <= 0, try to add the callout to
the sTimers list. However it is already in the list, and in many
circumstances the only item in the list, so it would get silently
linked to itself.
6. An infinite loop due to the looped linked-list and/or double-remove
resulting in NULL dereferences would result.
Steps 1-2 coinciding in just the right way was apparently very rare, hence
why this problem only appeared very infrequently. The code I wrote to force
their coinciding made the problem happen extremely frequently.
This fixes#18334.
(Figuring out that the problem was an item being linked to itself, and
most critically a double-*reschedule* not a double-*removal*, took
far too long to figure out. I will be refactoring this code more
in subsequent commits, but also introducing new assertions to our
linked-list systems which enabled me to finally track down this problem
at all. Who knows; perhaps they will shake loose other bugs.)
Use 0 as the magic value instead of a positive one. This way, <= 0
consistently signals that the callout is not in the list, whereas > 0
signals that it is.
* Reorganize inner loop for clarity and to reduce indentation.
* Handle callouts that are due at this exact system_time.
* "break" instead of "continue" if lock fails.
As far as I can tell, it has no consumers whatsoever outside the tree.
(wpa_supplicant did not even use it.) So, remove it altogether.
If that turns out to be mistaken, we can reinstate it temporarily
as a private class function or ABI-only symbol.
GetNextNetwork is very inefficient as it fetches all networks but only
returns one of them. GetNetworks was introduced to compensate for that,
but only the most regular consumers were initially migrated. Now,
the remaining consumers of the old API are converted to the new one.
The former will use the explicit preferred size if one is set;
otherwise, it will use the computed preferred size; whereas
the latter will only use the explicitly set one, which if it has
not been set, will just be an empty BRect.
Fixes#15434, #18329.
If callout_stop() runs at the same time the callout_thread is
about to process the callout, we can wind up with a situation where
the callout is "active" but has not yet run due to waiting on the mutex.
FreeBSD's documentation confirms that callout_stop must be called
(when "safe" is 0, i.e. not callout_drain) with the callout's lock
held, if it has one, and so we can prevent the callout from being
invoked at this point, too.
Additionally, fix return values of callout_reset.
May further help with #18315.
* TLS slots are not in any way architecture-specific; we do not even
have a mechanism by which they could be made so at present.
Thus, the initialization of them can be moved to common code, and
out of the per-architecture implementations.
* When dealing with a fork()ed thread, it will already have a TLS array
with values set in it. Therefore, do not overwrite the whole array,
but instead only update the specific values which have changed on fork.
This fixes at least part of #17896.
* Break in FlagsTypeHandler once we have cleared all flags.
* Use sizeof(hex) instead of writing out the size again.
* Place comment above variable name in patch_syscalls.
Our std::string is not broken; rather, the bytesRead may include the
terminating NULL byte, which was then added to the string and caused
misbehaviors. So we need to use strnlen with std::string also, and
then everything works as expected.
This way, we save 3 string comparisons on every non-empty return value.
Actually this code was broken before, and did not print error names
for return types other than status_t, because message.return_value
is an unsigned integer and thus "< 0" was always false. Now we cast
appropriately in the new TypeHandler, so this works properly.
From the FreeBSD manual pages:
> If the callout is currently being serviced and cannot be stopped,
> and at the same time a next invocation of the same callout is also
> scheduled, then callout_stop() unschedules the next run and returns
> zero.
Previously we would return zero but not unschedule the next run.
This may fix#18315.
Since we used a hash table with a fixed size (1024), collisions were
obviously inevitable, meaning that while insertions would always be
fast, lookups and deletions would take linear time to search the
linked-list for the area in question. For recently-created areas,
this would be fast; for less-recently-created areas, it would get
slower and slower and slower.
A particularly pathological case was the "mmap/24-1" test from the
Open POSIX Testsuite, which creates millions of areas until it hits
ENOMEM; it then simply exits, at which point it would run for minutes
and minutes in the kernel team deletion routines; how long I don't know,
as I rebooted before it finished.
This change fixes that problem, among others, at the cost of increased
area creation time, by using an AVL tree instead of a hash. For comparison,
mmap'ing 2 million areas with the "24-1" test before this change took
around 0m2.706s of real time, while afterwards it takes about 0m3.118s,
or around a 15% increase (1.152x).
On the other hand, the total test runtime for 2 million areas went from
around 2m11.050s to 0m4.035s, or around a 97% decrease (0.031x); in other
words, with this new code, it is *32 times faster.*
Area insertion will no longer be O(1), however, so the time increase
may go up with the number of areas present on the system; but if it's
only around 3 seconds to create 2 million areas, or about 1.56 us per area,
vs. 1.35 us before, I don't think that's worth worrying about.
My nonscientific "compile HaikuDepot with 2 cores in VM" benchmark
seems to be within the realm of "noise", anyway, with most results
both before and after this change coming in around 47s real time.
Change-Id: I230e17de4f80304d082152af83db8bd5abe7b831
This restores approximately the old fixed size from before the
previous commit. Ideally this would be the preferred size, not
the minimum size, but that isn't quite possible when contained in
a scroll view.
With B_MODAL_WINDOW, others can be brought in front. But with
B_MODAL_WINDOW_LOOK plus B_FLOATING_ALL_WINDOW_FEEL, it will
retain its current appearance, but always stay on top of all other
windows.
Since debug_server does not have a Deskbar entry, this will prevent
such windows from getting "lost" and then having to minimize all others
in order to find them (or forgetting about them altogether by accident.)
POSIX seems to be silent about whether this is permitted,
but at least FreeBSD explicitly does not allow it.
Fixes#16936.
Change-Id: If6b23410ae88245e706df77d276234f4775654f9
The kernel heap only uses object caches for objects up to size 8192.
Larger allocations have to go through the raw allocator. That can
get pretty expensive.
Adding instrumentation around the malloc/free calls in this function
showed that on my machine, some 596ms during boot were spent on
*malloc/free alone*, all else aside. After this change, we are at
around 110ms, or a >5x improvement. Running an fgrep -R on /system/
after boot increased the cumulative time in memory functions to over
5 seconds, while after this change it is "only" 1170ms.
Honestly, it seems like the object depots should be able to be faster
than that, even if this function is called thousands of times. But that
is a problem for a different investigation.
It would be even faster for every consumer of this data in
packagefs just allocated one set of buffers up front, or at least
for a single "read session", but plumbing that all the way
through the myriad abstractions of the Package Kit will
not be easy, and is left for another time, as well.
While the BSDs and glibc seem to have various _r functions,
they all return int for errors instead of a pointer, making ours
exactly backwards of theirs for error reporting and thus useless.
So, remove them from the header entirely. They are left in
for ABI backwards compatibility for the time being.
A few constants not used by anything in the tree (i.e. not actually
implemented by libnetwork/netresolv) were also dropped. Some deprecated
or non-standard functions were placed behind _DEFAULT_SOURCE or deleted
entirely.
The header itself is now organized approximately as the BSDs do,
although with various Haiku-isms instead of BSD-isms where appropriate.
This cuts out a lot of unnecessary protocol entires found in the
longer NetBSD (and originally from ISC) lists. This data is used
in the implementation of getservent(), etc.
The previous logic of _AddTrim plus _TrimNext meant that if a trim
range was specified in the final call to _TrimNext (i.e. with force=true),
but the trim buffer was already full at that point, that last range
would not actually be trimmed.
Now, _AddTrim returns true when the buffer is filled and trimming
should be done, rather than delaying it and requiring a second
add after trimming.
Change-Id: I4b782948e8dc9267c63e61bce0c078fd4e834177
Reviewed-on: https://review.haiku-os.org/c/haiku/+/6070
Tested-by: Automation <[email protected]>
Reviewed-by: waddlesplash <[email protected]>
Augustin Cavalier