Hello,
[email protected], le ven. 12 sept. 2025 13:55:35 +0200, a ecrit:
> @@ -572,9 +574,9 @@ io_select_common (struct trivfs_protid *cred,
>
> available = 0;
>
> + pthread_mutex_lock (&global_lock);
> while (1)
> {
> - pthread_mutex_lock (&global_lock);
> if ((*type & SELECT_READ) && buffer_readable (input_buffer))
> available |= SELECT_READ;
> if ((*type & SELECT_WRITE) &&
> @@ -588,13 +590,26 @@ io_select_common (struct trivfs_protid *cred,
> pthread_mutex_unlock (&global_lock);
> return 0;
> }
> -
> - if (cred->po->openmodes & O_NONBLOCK)
> + if (*type & SELECT_READ)
> {
> - pthread_mutex_unlock (&global_lock);
> - return EWOULDBLOCK;
> + err = start_input(0);
> + if (err)
> + {
> + *type = 0;
> + pthread_mutex_unlock (&global_lock);
> + return err;
> + }
> + }
Yes, that looks sensible.
> + if ((*type & SELECT_WRITE) && (cred->po->openmodes & O_WRITE))
> + {
> + err = start_output(0);
> + if (err)
> + {
> + *type = 0;
> + pthread_mutex_unlock (&global_lock);
> + return err;
> + }
> }
> -
> ports_interrupt_self_on_port_death (cred, reply);
> err = pthread_hurd_cond_timedwait_np (&select_alert, &global_lock,
> tsp);
> if (err)
> --8<---------------cut here---------------end--------------->8---
>
> I have no idea if the write part works and I just hope that it is
> symmetrical.
dev_write seems to work symmetrical with dev_read, so it'd probably be
correct path, yes.
> The condition in io_select_common explicitly checks for O_WRITE I am not sure
> if this is needed again because if it is not set select will return before.
It doesn't seem to be needed again indeed.
> It does not work at all if the pthread_mutex_lock is inside the while block.
Mmm, the mutex management indeed looks wrong: at the end of the while
loop it is not released. But that's unrelated to your issue, so we'd
want to commit that alone. Could you check that that modification alone
doesn't bring issues elsewhere on the system?
> All the timeout related things I tried seem to work as expected.
Good :)
> Finally the shepherd's kernel log also works with this if I additionally add
> this to mach
>
> diff --git a/device/kmsg.c b/device/kmsg.c
> index e5b518e6..4efa0046 100644
> --- a/device/kmsg.c
> +++ b/device/kmsg.c
> @@ -31,7 +31,7 @@ the Free Software Foundation, 675 Mass Ave, Cambridge, MA
> 02139, USA. */
> #include <device/kmsg.h>
>
>
> -#define KMSGBUFSIZE (4096) /* XXX */
> +#define KMSGBUFSIZE (8192) /* XXX */
I don't see why this should be needed? It's most probably rather just a
sign that something is wrong somewhere and needs to be fixed there,
rather than hiding the issue by increasing the buffer size.
Thanks for working on this!
Samuel
