Jan Kiszka wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Wait! When the sync object behind some file descriptor is deleted but
>>>>>> the descriptor itself is still existing, we rather have to return that
>>>>>> fd signaled from select() instead of letting the call fail. I beed to
>>>>>> look into this again.
>>>>> It looks to me like a transitory state, we can wait for the sync object
>>>>> to be deleted to have the fd destructor signaled. It should not be long.
>>>> That's not an issue of waiting for this. See e.g. TCP: peer closes
>>>> connection -> internal sync objects will be destroyed (to make
>>>> read/write fail). But the fd will remain valid until the local side
>>>> closes it as well.
>>> It looks to me like this is going to complicate things a lot, and will
>>> be a source of regressions. Why can we not have sync objects be
>>> destroyed when the fd is really destroyed and use a status bit of some
>>> kind to signal read/write that the fd was closed by peer?
>> It is way easier and more consistent to unblock reader and writers via
>> destroying the sync object than to signal it and add tests for specific
>> states to detect that. Keep in mind that this pattern is in use even
>> without select support. Diverging from it just to add select awareness
>> to some driver would be a step back.
>>
>
> First draft of a fix that so far does what it is supposed to do,
> comments welcome:
>
> diff --git a/ksrc/skins/posix/syscall.c b/ksrc/skins/posix/syscall.c
> index 959b61c..4e46cb6 100644
> --- a/ksrc/skins/posix/syscall.c
> +++ b/ksrc/skins/posix/syscall.c
> @@ -2298,24 +2298,30 @@ static int select_bind_one(struct xnselector
> *selector, unsigned type, int fd)
> }
>
> static int select_bind_all(struct xnselector *selector,
> - fd_set *fds[XNSELECT_MAX_TYPES], int nfds)
> + fd_set *in_fds[XNSELECT_MAX_TYPES],
> + fd_set *out_fds[XNSELECT_MAX_TYPES], int nfds)
> {
> unsigned fd, type;
> + int pending = 0;
> int err;
>
> for (type = 0; type < XNSELECT_MAX_TYPES; type++) {
> - fd_set *set = fds[type];
> + fd_set *set = in_fds[type];
> if (set)
> for (fd = find_first_bit(set->fds_bits, nfds);
> fd < nfds;
> fd = find_next_bit(set->fds_bits, nfds, fd + 1)) {
> err = select_bind_one(selector, type, fd);
> - if (err)
> - return err;
> + if (err) {
> + if (err != -EIDRM)
> + return err;
> + __FD_SET(fd, out_fds[type]);
> + pending++;
> + }
> }
> }
>
> - return 0;
> + return pending;
> }
>
> /* int select(int, fd_set *, fd_set *, fd_set *, struct timeval *) */
> @@ -2387,7 +2393,7 @@ static int __select(struct pt_regs *regs)
>
> /* Bind directly the file descriptors, we do not need to go
> through xnselect returning -ECHRNG */
> - if ((err = select_bind_all(selector, in_fds, nfds)))
> + if ((err = select_bind_all(selector, in_fds, out_fds, nfds)))
> return err;
> }
>
> @@ -2395,7 +2401,8 @@ static int __select(struct pt_regs *regs)
> err = xnselect(selector, out_fds, in_fds, nfds, timeout, mode);
>
> if (err == -ECHRNG) {
> - int err = select_bind_all(selector, out_fds, nfds);
> + int err = select_bind_all(selector, out_fds, out_fds,
> + nfds);
Mmh, this is probably no good idea, need to truly split in (aka "no yet
bound") and out (aka "pending") sets here as well.
> if (err)
> return err;
> }
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
