[Intel-gfx] [RFC PATCH 0/9] drm/i915/spi: discrete graphics internal spi

[Richard Weinberger]

)On Tue, Feb 16, 2021 at 7:26 PM Tomas Winkler <tomas.winkler at intel.com> wrote:
> Because the graphic card may undergo reset at any time and basically hot
> unplug all its child devices, this series also provides a fix to the mtd
> framework to make the reset graceful.

Well, just because MTD does not work as you expect, it is not broken. :-)

In your case i915_spi_remove() blindly removes the MTD, this is not allowed.
You may remove the MTD only if there are no more users.

The current model in MTD is that the driver is in charge of all life
cycle management.
Using ->_get_device() and ->_put_device() a driver can implement
refcounting and deny
new users if the MTD is about to disappear.

In the upcoming MUSE driver that mechanism is used too.
MUSE allows to implement a MTD in userspace. So the FUSE server can disappear at
*any* time. Just like in your case. Even worse, it can be hostile.
In MUSE the MTD life time is tied to the FUSE connection object,
muse_mtd_get_device()
increments the FUSE connection refcount, and muse_mtd_put_device()
decrements it.
That means if the FUSE server disappears all of a sudden but the MTD
still has users,
the MTD will stay. But in this state no new references are allowed and
all MTD operations
of existing users will fail with -ENOTCONN (via FUSE).
As soon the last user is gone (can be userspace via /dev/mtd* or a
in-kernel user such as UBIFS),
the MTD will be removed.
For the full details, please see:
https://git.kernel.org/pub/scm/linux/kernel/git/rw/misc.git/tree/fs/fuse/muse.c?h=muse_v3#n1034

Is in your case *really* not possible to do it that way?

On the other hand, your last patch moves some part of the life cycle
management into MTD core.
The MTD will stay as long it has users.
But that's only one part. The driver is still in charge to make sure
that all operations
fail immediately and that no new users arrive.
If we want to do all in MTD core we'd have to do it like SCSI disks.
That means having devices states such as SDEV_RUNNING, SDEV_CANCEL,
SDEV_OFFLINE, ....
That way the MTD could be shutdown gracefully, first no new users are
allowed, then ongoing operations
will be cancelled, next all operation will fail with -EIO or such,
then the device is being removed from sysfs
and finally if the last user is gone, the MTD can be removed.

I'm not sure whether we want to take that path.

-- 
Thanks,
//richard