linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	kvm <kvm@vger.kernel.org>
Subject: Re: [GIT PULL] VFIO fixes for v4.1-rc2
Date: Fri, 01 May 2015 16:03:39 -0600	[thread overview]
Message-ID: <1430517819.4472.331.camel@redhat.com> (raw)
In-Reply-To: <CA+55aFxc_RB8hPanU2d5f8MPGScq_4poP0hEuDO4sVjfw+KDeQ@mail.gmail.com>

On Fri, 2015-05-01 at 13:23 -0700, Linus Torvalds wrote:
> On Fri, May 1, 2015 at 11:48 AM, Alex Williamson
> <alex.williamson@redhat.com> wrote:
> >
> > Ok.  It seemed like useful behavior to be able to provide some response
> > to the user in the event that a ->remove handler is blocked by a device
> > in-use and the user attempts to abort the action.
> 
> Well, that kind of notification *might* be useful, but at the cost of
> saying "somebody tried to send you a signal, so I am now telling you
> about it, and then deleting that signal, and you'll never know what it
> actually was"?
> 
> That's not useful, that's just wrong.

Yep, it was a bad idea.

> Now, what might in theory be useful - but I haven't actually seen
> anybody do anything like that - is to start out with an interruptible
> sleep, warn if you get interrupted, and then continue with an
> un-interruptible sleep (leaving the signal active).

I was considering doing exactly this.

> But even that sounds like a very special case, and I don't think
> anything has ever done that.
> 
> In general, our signal handling falls into three distinct categories:
> 
>  (a) interruptible (and you can cancel the operation and return "try again")
> 
>  (b) killable (you can cancel the operation, knowing that the
> requester will be killed and won't try again)
> 
>  (c) uninterruptible
> 
> where that (b) tends to be a special case of an operation that
> technically isn't really interruptible (because the ABI doesn't allow
> for retrying or error returns), but knowing that the caller will never
> see the error case because it's killed means that you can do it. The
> classic example of that is an NFS mount that is mounted "nointr" - you
> can't return EINTR for a read or a write (because that invalidates
> POSIX) but you want to let SIGKILL just kill the process in the middle
> when the network is hung.

I think we're in that (c) case unless we want to change our driver API
to allow driver remove callbacks to return error.  Killing the caller
doesn't really help the situation without being able to back out of the
remove path.  Killing the task with the device open would help, but
seems rather harsh.  I expect we eventually want to be able to escalate
to revoking the device from the user, but currently we only have a
notifier to request the device from cooperative users.  In the event of
an uncooperative user, we block, which can be difficult to figure out,
especially when we're dealing with SR-IOV devices and a PF unbind
implicitly induces a VF unbind.  The interruptible component here is
simply a logging mechanism which should have turned into an
"interruptible_once" rather than a signal flush.

I try to avoid vfio being a special case, but maybe in this instance
it's worthwhile.  If you have other suggestions, please let me know.
Thanks,

Alex


  reply	other threads:[~2015-05-01 22:03 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-01 17:40 [GIT PULL] VFIO fixes for v4.1-rc2 Alex Williamson
2015-05-01 18:37 ` Linus Torvalds
2015-05-01 18:48   ` Alex Williamson
2015-05-01 20:23     ` Linus Torvalds
2015-05-01 22:03       ` Alex Williamson [this message]
2015-05-01 19:38   ` [PATCH] signals: Generate warning when flush_signals() is called from non-kthread context Ingo Molnar
2015-05-02  8:30     ` NeilBrown
2015-05-02 16:27       ` Linus Torvalds
2015-05-07 12:54         ` Peter Zijlstra
2015-05-04 17:35       ` Oleg Nesterov
2015-05-07 13:33         ` Jiri Kosina
2015-05-07 22:37           ` NeilBrown
2015-05-02 11:56     ` Evgeniy Polyakov
2015-05-02 16:33     ` Richard Weinberger
2015-05-03 17:34     ` Oleg Nesterov
2015-05-04 16:45       ` [PATCH 0/1] signals: don't abuse __flush_signals() in selinux_bprm_committed_creds() Oleg Nesterov
2015-05-04 16:45         ` [PATCH 1/1] " Oleg Nesterov
2015-05-04 19:43           ` Paul Moore
2015-05-06 10:19       ` [PATCH] signals: Generate warning when flush_signals() is called from non-kthread context Ingo Molnar
2015-05-01 20:11   ` [GIT PULL] VFIO fixes for v4.1-rc2 Richard Weinberger
2015-05-01 21:09     ` Richard Weinberger

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=1430517819.4472.331.camel@redhat.com \
    --to=alex.williamson@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=torvalds@linux-foundation.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).