public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <willy@w.ods.org>
To: Armin Schindler <aml@melware.de>
Cc: Linux Kernel Mailinglist <linux-kernel@vger.kernel.org>,
	Armin Schindler <armin@melware.de>
Subject: Re: [PATCH 2.4] sys_select() return error on bad file
Date: Thu, 4 Mar 2004 08:22:25 +0100	[thread overview]
Message-ID: <20040304072225.GA20915@alpha.home.local> (raw)
In-Reply-To: <Pine.LNX.4.31.0403030936570.9608-100000@phoenix.one.melware.de>

Hi,

On Wed, Mar 03, 2004 at 09:46:54AM +0100, Armin Schindler wrote:
> --- linux/fs/select.c_orig	2004-03-02 19:06:44.000000000 +0100
> +++ linux/fs/select.c	2004-03-03 09:25:24.000000000 +0100
> @@ -183,6 +183,8 @@
>  		wait = NULL;
>  	retval = 0;
>  	for (;;) {
> +		int file_err = 1;
> +

Just a thought, select() is often performance-critical, and adding one more
variable inside the loop can slow it down a bit. Wouldn't it be cheaper to
set retval to -EBADF above and avoid using file_err ?

>  		set_current_state(TASK_INTERRUPTIBLE);
>  		for (i = 0 ; i < n; i++) {
>  			unsigned long bit = BIT(i);
> @@ -199,6 +201,7 @@
>  						  i /*  The fd*/,
>  						  __timeout,
>  						  NULL);
> +				file_err = 0;

There you would put retval=0

>  				mask = DEFAULT_POLLMASK;
>  				if (file->f_op && file->f_op->poll)
>  					mask = file->f_op->poll(file, wait);
> @@ -227,6 +230,10 @@
>  			retval = table.error;
>  			break;
>  		}
> +		if (file_err) {
> +			retval = -EBADF;
> +			break;
> +		}

And there : if (retval == -EBADF) break;

>  		__timeout = schedule_timeout(__timeout);
>  	}
>  	current->state = TASK_RUNNING;

Just a thought anyway, I've not read the complete function.

Cheers,
Willy


  reply	other threads:[~2004-03-04  7:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-03-03  8:46 [PATCH 2.4] sys_select() return error on bad file Armin Schindler
2004-03-04  7:22 ` Willy Tarreau [this message]
2004-03-04  9:20   ` Armin Schindler
2004-03-04 13:48     ` Willy Tarreau
2004-03-14 15:58     ` Marcelo Tosatti
  -- strict thread matches above, loose matches on Subject: below --
2004-03-14 18:18 Manfred Spraul
2004-03-15 10:33 ` Armin Schindler

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=20040304072225.GA20915@alpha.home.local \
    --to=willy@w.ods.org \
    --cc=aml@melware.de \
    --cc=armin@melware.de \
    --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