Re: [PATCH 2.6.24] block2mtd: removing a device and typo fixes

[Stephane Chazelas]

2008-02-19 16:08:22 +0100, Jörn Engel:
> [ Just returned home. ]

Hi Jörn,

I've got to admit I've been a little too busy to look at it
myself. I'll send a patch for the obvious fixes soon though.

[...]
> > Well, yes that raised a concern to me, the "exit" function
> > returns "void". If the del_mtd_device fails with EBUSY at the
> > moment (such as when a /dev/mtdblock<x> is mounted), we ignore
> > it and go on with freeing everything leaving a dangling mtd.
> > 
> > Is there a way around that?
> 
> For module unload we should be safe.  Errors are only returned if the
> device doesn't exist (a clear bug) or if the device is still being used.
> Module refcounting should prevent the latter case, so either way we have
> a bug if del_mtd_device returns an error.

At the moment, when we bind a mtd device to a block device, we
don't increase the refcount. When a mtdblock on a block2mtd, the
refcount is not increased either (the mtdblock's one is I
guess).

When you unload block2mtd, the block2mtd exit code tries and
remove the mtd bindings. If it fails, it exits anyway, leaving
some dangling stuff. That's why I suggested that the exit code
doesn't do the cleaning but that the requirement of cleaning
things up is  put upon the user before he is allowed to unload
the module (increasing the refcount every time block2mtd is used
to map a mtd to a block device).

> When removing the device via your proposed interface we should simply
> return the error to userspace, if the interface permits.  Sysfs doesn't
> seem to permit error returns, so we should keep the device alive and do
> nothing.

Agreed, that's what I was doing in my patch. But I wasn't doing
anything with the refcount, this was still handled as before,
that is we relied on the exit code to do the clean up.

Best regards,
Stephane
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/