public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ingo Molnar <mingo@elte.hu>, Al Viro <viro@ZenIV.linux.org.uk>,
	hch@infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Eric Van Hensbergen <ericvh@gmail.com>,
	Ron Minnich <rminnich@sandia.gov>,
	v9fs-developer@lists.sourceforge.net,
	Takashi Iwai <tiwai@suse.de>
Subject: Re: [PATCH 2/2] poll: allow f_op->poll to sleep
Date: Tue, 26 Aug 2008 19:48:17 +0200	[thread overview]
Message-ID: <48B441E1.5090304@gmail.com> (raw)
In-Reply-To: <alpine.LFD.1.10.0808260954040.3363@nehalem.linux-foundation.org>

(cc'ing Takashi Iwai)

Hello, Linus.

Linus Torvalds wrote:
>> This patch converts poll/select to use custom wake up function and use
>> separate triggered variable to synchronize against wake up events.
>> The only added overhead is an extra function call during wake up and
>> negligible.
> 
> I don't really see the point.
> 
> poll() isn't allowed to sleep for many reasons. Some are technical. But 
> the most obvious one is that a sleeping "poll()" is totally against the 
> whole point of polling in the first place!
> 
> So is there some big conceptual reason to change how poll() has always 
> worked?

No big conceptual reason at all.  I was trying to implement poll for
FUSE so that character device can be implemented in userspace such that
/dev/dsp can be routed to pulseaudio (or whatever) and for userland
filesystem, non-sleep poll just couldn't be made to work and there
didn't seem to be no big reason for poll not being able to sleep, so...

The OSS sound routing is probably a bit too late but the exclusion
between OSS and the modern sound system is still quite annoying and for
simple sound apps OSS interface is just much simpler as it's just normal
file IOs and fully contained in kernel (e.g. arch-um host sound support).

> If you worry about debuggability, then we could just add a 
> 
> 	preempt_enable();
> 	..
> 	preempt_disable();
> 
> around the poll calls (purely for catching errors) to get a big warning if 
> somebody tries to call a sleepable function.

Yeap, that will be a nice debug option to have if this patch gets NACKed.

Thanks.

-- 
tejun

  reply	other threads:[~2008-08-26 17:49 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-26 13:57 [PATCH 1/2] wait: kill is_sync_wait() Tejun Heo
2008-08-26 13:59 ` [PATCH 2/2] poll: allow f_op->poll to sleep Tejun Heo
2008-08-26 16:58   ` Linus Torvalds
2008-08-26 17:48     ` Tejun Heo [this message]
2008-08-26 19:51     ` Christoph Hellwig
2008-08-26 20:22     ` Ingo Molnar
2008-08-26 14:09 ` [PATCH 1/2] wait: kill is_sync_wait() Ingo Molnar

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=48B441E1.5090304@gmail.com \
    --to=htejun@gmail.com \
    --cc=ericvh@gmail.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rminnich@sandia.gov \
    --cc=tiwai@suse.de \
    --cc=torvalds@linux-foundation.org \
    --cc=v9fs-developer@lists.sourceforge.net \
    --cc=viro@ZenIV.linux.org.uk \
    /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