linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Constantine Gavrilov <const-g@optibase.com>
To: Dan Maas <dmaas@maasdigital.com>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: Problems implementing poll call
Date: Sun, 20 Oct 2002 18:20:12 +0200	[thread overview]
Message-ID: <3DB2D7BC.5080809@optibase.com> (raw)

Thanks a lot. I get it now.

As far as races are concerned,  it should be OK. This is because my 
condition uses testbit() and interrupt handler uses setbit(). These 
operations are atomic even on SMP configurations. Right?

What do you think is of lower latency -- a poll() hook or an ioctl() hook?

One thing I could have with ioctl implementation was that I knew in 
kernel space that a timeout has occurred. With poll(), I do not know how 
to catch an error, because the poll function is called many times and I 
do not know which return is last. But it is not critical for me.

Dan Maas wrote:

>Hi Constantine, this is in reponse to your post on the Linux kernel
>list of 17 October...
>
>I think you should take a look at the poll() implementation of other
>drivers. You should call poll_wait() before testing the bit flag,
>regardless of whether you actually need to block or not.
>
>Note that poll_wait() does not block; it merely adds your device's
>wait queue to a list of items the user is waiting on. The actual
>blocking occurs in other kernel code after your poll() method
>returns... select() and poll() can make several passes over the file
>descriptors; wait_table may NULL if the kernel knows for sure it is
>not going to block and is just checking the status of your device.
>
>I think you also need to be a little more careful about race
>conditions in your sleeping code. If you write this:
>
>if(need_to_sleep) {
>	interruptible_sleep_on(...);
>}
>
>You create a potential race condition where need_to_sleep is changed
>by an interrupt between the if() check and the sleep_on(). To
>eliminate these races you need to set the task state to
>TASK_INTERRUPTIBLE, then check the 'need_to_sleep' condition, then
>schedule() to actually go to sleep. This way if the interrupt handler
>changes need_to_sleep after your if() statement, you will wake up
>immediately instead of staying asleep forever. For an example of this
>technique see the comments around line 1680 in my driver
>drivers/ieee1394/dv1394.c
>
>Regards,
>Dan
>


-- 
----------------------------------------
Constantine Gavrilov
Linux Leader
Optibase Ltd
7 Shenkar St, Herzliya 46120, Israel
Phone: (972-9)-970-9140
Fax:   (972-9)-958-6099
----------------------------------------




             reply	other threads:[~2002-10-20 16:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-10-20 16:20 Constantine Gavrilov [this message]
2002-10-20 20:31 ` Problems implementing poll call Dan Maas
  -- strict thread matches above, loose matches on Subject: below --
2002-10-17 11:12 Constantine Gavrilov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3DB2D7BC.5080809@optibase.com \
    --to=const-g@optibase.com \
    --cc=dmaas@maasdigital.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).