linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vitaly Luban <vitaly@luban.org>
To: Dan Kegel <dank@kegel.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: spin_lock_bh() usage check, please (was: [PATCH][RFC] Signal-per-fd  for RT signals)
Date: Sat, 15 Sep 2001 14:20:27 -0700	[thread overview]
Message-ID: <3BA3C61A.DED5A27A@luban.org> (raw)
In-Reply-To: <3BA2AFFF.C7B8C4DF@kegel.com> <3BA2E144.FB0E5D55@luban.org> <3BA2E99A.1134E382@kegel.com> <3BA350A7.7D39FC23@kegel.com>

Dan Kegel wrote:

> so it crashes on a uniprocessor system when an interrupt that
> causes a sigio comes in while the process was busy expanding its
> file table, to wit:

I'm checking that. My variant of patch will be posted tomorrow, after I'll
finish testing it. It will be against 2.4.9 and addresses all crashes you've
mentioned.


>
> p.s. The bug was introduced by Luban's one-signal-per-fd patch; I ran into
> it while running my variant of his patch, http://www.kegel.com/linux/onefd-dank.patch
> My variant makes the following changes:
> * it fixes an oops related to 'task->files' being null during SIGINT
> * it fixes two oopses related to signal queue overflow
> * it keeps poll data, not pointers, in struct file.  This saves space
>   and makes the consequence of screwing up less severe.
> * it overloads an existing struct file field to avoid the space penalty;
>   it doesn't bloat struct file.
> * it assumes that it's better to keep the kernel uncluttered, so it
>   changes the meaning of siginfo.si_code rather than introduces new ioctl's.
>   (I hear the Austin draft of the Posix single unix spec is deleting all mention
>    of SIGIO, so it looks like we're free to 'improve' that interface freely
>    once Austin becomes the law of the land :-)

I'm reluctant to make such changes, like f_error field overloading because
a) it may backbite in future if you'll use this mechanism for all I/O and not only
sockets,
since you are interfering with it's legitimate usage, in short, it makes patch more
dependent of possible kernel code changes
b) I want the default kernel behavior to be exactly the same, as if without this
patch, thus additional fcntl to control this feature. If you do not activate it, you
don't get it,

Keeping pointer allows for additional sanity check to make sure I'm not screwed,
and cheap update events information in siginfo.
Keeping interests strips you of this ability. And also, you have to have additional
method of gettong events information in user space, because you are not updating
siginfo,
and cannot rely on it anymore. So, to utilize signal per fd feature you have to modify
event loop and not file descriptor setup, which I'd also try to avoid.

Thanks,
    Vitaly.



  reply	other threads:[~2001-09-15 21:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-09-15  1:33 [PATCH][RFC] Signal-per-fd for RT signals Dan Kegel
2001-09-15  5:04 ` Vitaly Luban
2001-09-15  5:39   ` Dan Kegel
2001-09-15 12:59     ` spin_lock_bh() usage check, please (was: [PATCH][RFC] Signal-per-fd for RT signals) Dan Kegel
2001-09-15 21:20       ` Vitaly Luban [this message]
2001-09-15 22:07         ` Dan Kegel
2001-09-16  2:35           ` [PATCH][RFC] Signal-per-fd for RT signals Vitaly Luban
2001-09-16  3:51             ` Dan Kegel
2001-09-22 23:30             ` [PATCH][RFC] Signal-per-fd for RT signals; write_lock_bh(file_lock)? Dan Kegel

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=3BA3C61A.DED5A27A@luban.org \
    --to=vitaly@luban.org \
    --cc=dank@kegel.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).