Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jani Nikula <jani.nikula@linux.intel.com>
To:     Chris Wilson <chris@chris-wilson.co.uk>,
        Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc:     David Airlie <airlied@linux.ie>, intel-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix context IDs not released on driver hot unbind
In-Reply-To: <155437522546.7532.6754257066058816161@skylake-alporthouse-com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
References: <20190404102445.12303-1-janusz.krzysztofik@linux.intel.com> <155437373474.7532.10868620123516507965@skylake-alporthouse-com> <c1a857b241020a44c070cfc6b57adb6e6aba00be.camel@linux.intel.com> <155437462649.7532.18347010454266779928@skylake-alporthouse-com> <1bda752c136f3a75817fe257027edd8be4e7472e.camel@linux.intel.com> <155437522546.7532.6754257066058816161@skylake-alporthouse-com>
Date:   Thu, 04 Apr 2019 16:47:31 +0300
Message-ID: <87r2ahyj9o.fsf@intel.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 04 Apr 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Janusz Krzysztofik (2019-04-04 11:50:14)
>> On Thu, 2019-04-04 at 11:43 +0100, Chris Wilson wrote:
>> > Quoting Janusz Krzysztofik (2019-04-04 11:40:24)
>> > > On Thu, 2019-04-04 at 11:28 +0100, Chris Wilson wrote:
>> > > > Quoting Janusz Krzysztofik (2019-04-04 11:24:45)
>> > > > > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
>> > > > > 
>> > > > > In case the driver gets unbound while a device is open, kernel
>> > > > > panic
>> > > > > may be forced if a list of allocated context IDs is not empty.
>> > > > > 
>> > > > > When a device is open, the list may happen to be not empty
>> > > > > because
>> > > > > a
>> > > > > context ID, once allocated by a context ID allocator to a
>> > > > > context
>> > > > > assosiated with that open file descriptor, is released as late
>> > > > > as
>> > > > > on device close.
>> > > > > 
>> > > > > On the other hand, there is a need to release all allocated
>> > > > > context
>> > > > > IDs
>> > > > > and destroy the context ID allocator on driver unbind, even if
>> > > > > a
>> > > > > device
>> > > > > is open, in order to free memory resources consumed and prevent
>> > > > > from
>> > > > > memory leaks.  The purpose of the forced kernel panic was to
>> > > > > protect
>> > > > > the context ID allocator from being silently destroyed if not
>> > > > > all
>> > > > > allocated IDs had been released.
>> > > > 
>> > > > Those open fd are still pointing into kernel memory where the
>> > > > driver
>> > > > used to be. The panic is entirely correct, we should not be
>> > > > unloading
>> > > > the module before those dangling pointers have been made safe.
>> > > > 
>> > > > This is papering over the symptom. How is the module being
>> > > > unloaded
>> > > > with
>> > > > open fd? 
>> > > 
>> > > A user can play with the driver unbind or device remove sysfs
>> > > interface.
>> > 
>> > Sure, but we must still follow all the steps before _unloading_ the
>> > module or else the user is left pointing into reused kernel memory.
>> 
>> I'm not talking about unloading the module, that is prevented by open
>> fds.  The driver still exists after being unbound from a device and may
>> just respond with -ENODEV.
>
> i915_gem_contexts_fini() *is* module unload.

Janusz, please describe what you're doing exactly.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center