RE: select() unblocks when no data to read()

RE: select() unblocks when no data to read()

[Jeff Webb]

I am having trouble using select() to perform a timed wait on a UDD file descriptor to detect interrupts from a custom PCI card.  After some searching, I came across this thread from 2017 that describes my exact issue:

https://xenomai.org/pipermail/xenomai/2017-July/037494.html

I am using x86-64 instead of ARM, but I am having the same trouble.  The original poster did a great job of describing the problem, so I won't repeat it.  If I use only read() without select(), things work as expected, but I need to implement a timeout.

It sounds like Philippe was going to look into the issue further.  Has any progress on this issue been made since the last post in this thread?

Thanks,

-Jeff Webb

Re: select() unblocks when no data to read()

[Greg Gallagher]

On Tue, May 7, 2019 at 9:54 PM Jeff Webb via Xenomai
<xenomai@xenomai.org> wrote:
>
> I am having trouble using select() to perform a timed wait on a UDD file descriptor to detect interrupts from a custom PCI card.  After some searching, I came across this thread from 2017 that describes my exact issue:
>
> https://xenomai.org/pipermail/xenomai/2017-July/037494.html
>
> I am using x86-64 instead of ARM, but I am having the same trouble.  The original poster did a great job of describing the problem, so I won't repeat it.  If I use only read() without select(), things work as expected, but I need to implement a timeout.
>
> It sounds like Philippe was going to look into the issue further.  Has any progress on this issue been made since the last post in this thread?
>
> Thanks,
>
> -Jeff Webb
>
>

There was a fix posted to the list a while ago but it wasn't a patch
it was the whole c file.  I don't think a patch was ever made, I'll
see if I can dig it up.

-Greg

Re: select() unblocks when no data to read()

[Jeff Webb]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, May 9, 2019 9:19 AM, Greg Gallagher <greg@embeddedgreg.com> wrote:

> On Tue, May 7, 2019 at 9:54 PM Jeff Webb via Xenomai
> xenomai@xenomai.org wrote:
>
> > I am having trouble using select() to perform a timed wait on a UDD file descriptor to detect interrupts from a custom PCI card. After some searching, I came across this thread from 2017 that describes my exact issue:
> > https://xenomai.org/pipermail/xenomai/2017-July/037494.html
> > I am using x86-64 instead of ARM, but I am having the same trouble. The original poster did a great job of describing the problem, so I won't repeat it. If I use only read() without select(), things work as expected, but I need to implement a timeout.
> > It sounds like Philippe was going to look into the issue further. Has any progress on this issue been made since the last post in this thread?
> > Thanks,
> > -Jeff Webb
>
> There was a fix posted to the list a while ago but it wasn't a patch
> it was the whole c file. I don't think a patch was ever made, I'll
> see if I can dig it up.

Thanks for looking into it, Greg.  I must have missed that post.  The original poster's solution of adding this line:

  rtdm_event_clear(&ur->pulse);

just before the return statement in udd_read_rt() is working for me.  Philippe mentioned that this might be racy, but maybe it's okay (or maybe not?) if there are no shared interrupts and interrupts are unmasked by the userspace application after the read completes.

-Jeff

Re: select() unblocks when no data to read()

[Greg Gallagher]

On Thu, May 9, 2019 at 9:26 PM Jeff Webb <w1@codecraftsmen.org> wrote:
>
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Thursday, May 9, 2019 9:19 AM, Greg Gallagher <greg@embeddedgreg.com> wrote:
>
> > On Tue, May 7, 2019 at 9:54 PM Jeff Webb via Xenomai
> > xenomai@xenomai.org wrote:
> >
> > > I am having trouble using select() to perform a timed wait on a UDD file descriptor to detect interrupts from a custom PCI card. After some searching, I came across this thread from 2017 that describes my exact issue:
> > > https://xenomai.org/pipermail/xenomai/2017-July/037494.html
> > > I am using x86-64 instead of ARM, but I am having the same trouble. The original poster did a great job of describing the problem, so I won't repeat it. If I use only read() without select(), things work as expected, but I need to implement a timeout.
> > > It sounds like Philippe was going to look into the issue further. Has any progress on this issue been made since the last post in this thread?
> > > Thanks,
> > > -Jeff Webb
> >
> > There was a fix posted to the list a while ago but it wasn't a patch
> > it was the whole c file. I don't think a patch was ever made, I'll
> > see if I can dig it up.
>
> Thanks for looking into it, Greg.  I must have missed that post.  The original poster's solution of adding this line:
>
>   rtdm_event_clear(&ur->pulse);
>
> just before the return statement in udd_read_rt() is working for me.  Philippe mentioned that this might be racy, but maybe it's okay (or maybe not?) if there are no shared interrupts and interrupts are unmasked by the userspace application after the read completes.
>
> -Jeff
>

Here's the thread:
https://www.xenomai.org/pipermail/xenomai/2018-May/038934.html

I'm not sure if that will solve your issue, I can make a patch
tomorrow if that's easier and you can test it out.  I haven't dug to
far into this but I don't mind taking a look if that will help.

-Greg

Re: select() unblocks when no data to read()

[Philippe Gerum]

On 5/10/19 3:26 AM, Jeff Webb via Xenomai wrote:
> 
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Thursday, May 9, 2019 9:19 AM, Greg Gallagher <greg@embeddedgreg.com> wrote:
> 
>> On Tue, May 7, 2019 at 9:54 PM Jeff Webb via Xenomai
>> xenomai@xenomai.org wrote:
>>
>>> I am having trouble using select() to perform a timed wait on a UDD file descriptor to detect interrupts from a custom PCI card. After some searching, I came across this thread from 2017 that describes my exact issue:
>>> https://xenomai.org/pipermail/xenomai/2017-July/037494.html
>>> I am using x86-64 instead of ARM, but I am having the same trouble. The original poster did a great job of describing the problem, so I won't repeat it. If I use only read() without select(), things work as expected, but I need to implement a timeout.
>>> It sounds like Philippe was going to look into the issue further. Has any progress on this issue been made since the last post in this thread?
>>> Thanks,
>>> -Jeff Webb
>>
>> There was a fix posted to the list a while ago but it wasn't a patch
>> it was the whole c file. I don't think a patch was ever made, I'll
>> see if I can dig it up.
> 
> Thanks for looking into it, Greg.  I must have missed that post.  The original poster's solution of adding this line:
> 
>   rtdm_event_clear(&ur->pulse);
> 
> just before the return statement in udd_read_rt() is working for me.  Philippe mentioned that this might be racy, but maybe it's okay (or maybe not?) if there are no shared interrupts and interrupts are unmasked by the userspace application after the read completes.
> 

There is a logic flaw on entry to read_rt, when an interrupt already occurred before we consider waiting for the next one; in that case, the selector block is not reset as it should, which would explain why you get those spurious select() events. Could you try the following change?

Thanks,

diff --git a/include/cobalt/kernel/rtdm/udd.h b/include/cobalt/kernel/rtdm/udd.h
index 550ba9d24..bc2a68db6 100644
--- a/include/cobalt/kernel/rtdm/udd.h
+++ b/include/cobalt/kernel/rtdm/udd.h
@@ -306,7 +306,7 @@ struct udd_device {
 	/** Reserved to the UDD core. */
 	struct udd_reserved {
 		rtdm_irq_t irqh;
-		atomic_t event;
+		u32 event_count;
 		struct udd_signotify signfy;
 		struct rtdm_event pulse;
 		struct rtdm_driver driver;
diff --git a/kernel/drivers/udd/udd.c b/kernel/drivers/udd/udd.c
index 972db1225..b086096cc 100644
--- a/kernel/drivers/udd/udd.c
+++ b/kernel/drivers/udd/udd.c
@@ -117,7 +117,8 @@ static ssize_t udd_read_rt(struct rtdm_fd *fd,
 	struct udd_context *context;
 	struct udd_reserved *ur;
 	struct udd_device *udd;
-	ssize_t ret;
+	rtdm_lockctx_t ctx;
+	ssize_t ret = 0;
 	u32 count;
 
 	if (len != sizeof(count))
@@ -130,15 +131,20 @@ static ssize_t udd_read_rt(struct rtdm_fd *fd,
 	ur = &udd->__reserved;
 	context = rtdm_fd_to_private(fd);
 
-	for (;;) {
-		if (atomic_read(&ur->event) != context->event_count)
-			break;
+	cobalt_atomic_enter(ctx);
+
+	if (ur->event_count != context->event_count)
+		rtdm_event_clear(&ur->pulse);
+	else
 		ret = rtdm_event_wait(&ur->pulse);
-		if (ret)
-			return ret;
-	}
 
-	count = atomic_read(&ur->event);
+	count = ur->event_count;
+
+	cobalt_atomic_leave(ctx);
+
+	if (ret)
+		return ret;
+
 	context->event_count = count;
 	ret = rtdm_copy_to_user(fd, buf, &count, sizeof(count));
 
@@ -404,7 +410,7 @@ int udd_register_device(struct udd_device *udd)
 	} else
 		ur->mapper_name = NULL;
 
-	atomic_set(&ur->event, 0);
+	ur->event_count = 0;
 	rtdm_event_init(&ur->pulse, 0);
 	ur->signfy.pid = -1;
 
@@ -501,12 +507,15 @@ void udd_notify_event(struct udd_device *udd)
 {
 	struct udd_reserved *ur = &udd->__reserved;
 	union sigval sival;
+	rtdm_lockctx_t ctx;
 
-	atomic_inc(&ur->event);
+	cobalt_atomic_enter(ctx);
+	ur->event_count++;
 	rtdm_event_signal(&ur->pulse);
+	cobalt_atomic_leave(ctx);
 
 	if (ur->signfy.pid > 0) {
-		sival.sival_int = atomic_read(&ur->event);
+		sival.sival_int = (int)ur->event_count;
 		__cobalt_sigqueue(ur->signfy.pid, ur->signfy.sig, &sival);
 	}
 }

-- 
Philippe.

Re: select() unblocks when no data to read()

[Jeff Webb]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, May 9, 2019 8:37 PM, Greg Gallagher <greg@embeddedgreg.com> wrote:

> On Thu, May 9, 2019 at 9:26 PM Jeff Webb w1@codecraftsmen.org wrote:
>
> > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> > On Thursday, May 9, 2019 9:19 AM, Greg Gallagher greg@embeddedgreg.com wrote:
> >
> > > On Tue, May 7, 2019 at 9:54 PM Jeff Webb via Xenomai
> > > xenomai@xenomai.org wrote:
> > >
> > > > I am having trouble using select() to perform a timed wait on a UDD file descriptor to detect interrupts from a custom PCI card. After some searching, I came across this thread from 2017 that describes my exact issue:
> > > > https://xenomai.org/pipermail/xenomai/2017-July/037494.html
> > > > I am using x86-64 instead of ARM, but I am having the same trouble. The original poster did a great job of describing the problem, so I won't repeat it. If I use only read() without select(), things work as expected, but I need to implement a timeout.
> > > > It sounds like Philippe was going to look into the issue further. Has any progress on this issue been made since the last post in this thread?
> > > > Thanks,
> > > > -Jeff Webb
> > >
> > > There was a fix posted to the list a while ago but it wasn't a patch
> > > it was the whole c file. I don't think a patch was ever made, I'll
> > > see if I can dig it up.
> >
> > Thanks for looking into it, Greg. I must have missed that post. The original poster's solution of adding this line:
> > rtdm_event_clear(&ur->pulse);
> > just before the return statement in udd_read_rt() is working for me. Philippe mentioned that this might be racy, but maybe it's okay (or maybe not?) if there are no shared interrupts and interrupts are unmasked by the userspace application after the read completes.
> > -Jeff
>
> Here's the thread:
> https://www.xenomai.org/pipermail/xenomai/2018-May/038934.html
>
> I'm not sure if that will solve your issue, I can make a patch
> tomorrow if that's easier and you can test it out. I haven't dug to
> far into this but I don't mind taking a look if that will help.
>
> -Greg

Thanks for digging this up, Greg.  It looks like I already have the one-line udd_read_rt() part of this solution, and it works.  I am not using the write functionality, so I didn't test that.  It also looks like Philippe's patch works as well.

Thanks again,

-Jeff

Re: select() unblocks when no data to read()

[Jeff Webb]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, May 10, 2019 3:24 AM, Philippe Gerum <rpm@xenomai.org> wrote:

> On 5/10/19 3:26 AM, Jeff Webb via Xenomai wrote:
>
> > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> > On Thursday, May 9, 2019 9:19 AM, Greg Gallagher greg@embeddedgreg.com wrote:
> >
> > > On Tue, May 7, 2019 at 9:54 PM Jeff Webb via Xenomai
> > > xenomai@xenomai.org wrote:
> > >
> > > > I am having trouble using select() to perform a timed wait on a UDD file descriptor to detect interrupts from a custom PCI card. After some searching, I came across this thread from 2017 that describes my exact issue:
> > > > https://xenomai.org/pipermail/xenomai/2017-July/037494.html
> > > > I am using x86-64 instead of ARM, but I am having the same trouble. The original poster did a great job of describing the problem, so I won't repeat it. If I use only read() without select(), things work as expected, but I need to implement a timeout.
> > > > It sounds like Philippe was going to look into the issue further. Has any progress on this issue been made since the last post in this thread?
> > > > Thanks,
> > > > -Jeff Webb
> > >
> > > There was a fix posted to the list a while ago but it wasn't a patch
> > > it was the whole c file. I don't think a patch was ever made, I'll
> > > see if I can dig it up.
> >
> > Thanks for looking into it, Greg. I must have missed that post. The original poster's solution of adding this line:
> > rtdm_event_clear(&ur->pulse);
> > just before the return statement in udd_read_rt() is working for me. Philippe mentioned that this might be racy, but maybe it's okay (or maybe not?) if there are no shared interrupts and interrupts are unmasked by the userspace application after the read completes.
>
> There is a logic flaw on entry to read_rt, when an interrupt already occurred before we consider waiting for the next one; in that case, the selector block is not reset as it should, which would explain why you get those spurious select() events. Could you try the following change?

Thank you for putting together this patch, Philippe.  Sorry for the delay in testing it, but it seems to work.  I tested it on Xenomai 3.0.5, but it appears there are no other significant changes to UDD since then.  I will keep using your patch and see if any more subtle issues arise.  I will also test with the latest Xenomai when I get a chance.  Thank you again for taking the time to look into this.

-Jeff