Re: [PATCH 1/7] userfaultfd: require UFFDIO_API before other ioctls

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: "Huangpeng (Peter)" <peter.huangpeng@huawei.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, Pavel Emelyanov <xemul@parallels.com>,
	Hugh Dickins <hughd@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Andres Lagar-Cavilla <andreslc@google.com>,
	Andy Lutomirski <luto@amacapital.net>,
	linux-mm@kvack.org, Johannes Weiner <hannes@cmpxchg.org>,
	Rik van Riel <riel@redhat.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	linux-kernel@vger.kernel.org, zhang.zhanghailiang@huawei.com,
	Sanidhya Kashyap <sanidhya.gatech@gmail.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Peter Feiner <pfeiner@google.com>, Mel Gorman <mgorman@suse.de>,
	kvm@vger.kernel.org
Subject: Re: [PATCH 1/7] userfaultfd: require UFFDIO_API before other ioctls
Date: Mon, 15 Jun 2015 08:11:50 -1000	[thread overview]
Message-ID: <CA+55aFzdZJw7Ot7=PYyyskNhkv=H+NPzoF6rKtb6oMyzkuQ-=Q@mail.gmail.com> (raw)
In-Reply-To: <1434388931-24487-2-git-send-email-aarcange@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

On Jun 15, 2015 7:22 AM, "Andrea Arcangeli" <aarcange@redhat.com> wrote:
>
> +       if (cmd != UFFDIO_API) {
> +               if (ctx->state == UFFD_STATE_WAIT_API)
> +                       return -EINVAL;
> +               BUG_ON(ctx->state != UFFD_STATE_RUNNING);
> +       }

NAK.

Once again: we don't add BUG_ON() as some kind of assert. If your
non-critical code has s bug in it, you do WARN_ONCE() and you return. You
don't kill the machine just because of some "this can't happen" situation.

It turns out "this can't happen" happens way too often, just because code
changes, or programmers didn't think all the cases through. And killing the
machine is just NOT ACCEPTABLE.

People need to stop adding machine-killing checks to code that just doesn't
merit killing the machine.

And if you are so damn sure that it really cannot happen ever, then you
damn well had better remove the test too!

BUG_ON is not a debugging tool, or a "I think this would be bad" helper.

    Linus

[-- Attachment #2: Type: text/html, Size: 1338 bytes --]