Mbed-os: init/deinit conflict among utility block devices on the same SD card

[ccli8]

Description

Utility block devices (ChainingBlockDevice/SlicingBlockDevice/MBRBlockDevice) are constructed upon e.g. SDBlockDevice. If multiple utility block devices are constructed on the same underlying block device, there would be init/deinit conflict among them. Take SlicingBlockDevice as an example: its init/deinit would call init/deinit of the underlying block device (_bd).

int SlicingBlockDevice::init()
{
    int err = _bd->init();

int SlicingBlockDevice::deinit()
{
    return _bd->deinit();

I can think of two issues with this:

1. init of the underlying block device would be called multiple times by utility block devices constructed on it.
2. Calling deinit of the underlying block device in one utility block device would fail another utility block device constructed on the same underlying block device.

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

Related issue

https://github.com/ARMmbed/simple-mbed-cloud-client/issues/13

[ciarmcom]

ARM Internal Ref: MBOTRIAGE-1159

[davidsaada]

This is indeed problematic. Had similar problems when using chained BDs or when BD was included as a member of a class.
Maybe we should have an init/deinit reference count. @geky What do you think?

[geky]

Agreed. I've noticed this problem for the SlicingBlockDevice/MBRBlockDevice since they share a single resource, but is it an issue for the ChainingBlockDevice? I don't think we end up with multiple ChainingBlockDevice's using one resource.

Reference counting init/deinit would solve the problem, but unfortunately it would need to be in the block device drivers.

Note that a lot of the block device drivers are stateless and calling init multiple times has no side effects. I believe spif, dataflash, and i2ceeprom all fall into this category.

cc @deepikabhavnani

[deepikabhavnani]

calling deinit of the underlying block device in one utility block device would fail another utility block device constructed on the same underlying block device

Calling init multiple times should not have side effects for SD driver as well. deinit will definitely be problem https://github.com/ARMmbed/sd-driver/blob/master/SDBlockDevice.cpp#L403

Maybe we should have an init/deinit reference count.

Yes reference count along with existing init flag _is_initialized in SD driver can resolve this.

[davidsaada]

Having an initialized variable for the block devices should solve the multiple init problem. As for deinit - maybe we should not call it automatically from the calling block device.

[davidsaada]

OK, will handle it.

[davidsaada]

Yes reference count along with existing init flag _is_initialized in SD driver can resolve this.

@deepikabhavnani Why would you need two variables for it? A value of 0 in the ref count means that the device is uninitialized, while all other values means it is initialized. In this case, the _is_initialized is redundant.

[deepikabhavnani]

@davidsaada - Yes we can use reference count to check the device initialization status as well.. I just wanted to point that we have _is_initialized functionality as well which should be taken care of.

[davidsaada]

Implemented #7468 for all internal mbed-os BDs. Will handle the external ones (SD, SPIF etc.) once this PR is merged (as the init_ref_count variable has been added to the base class).

[davidsaada]

@0xc0170 Maybe this was prematurely closed. I still need to implement this on the SD block device (and other external BDs such as SPIF). #7468 only implements it for the internal mbed-os block devices.

[0xc0170]

It was automatically. Reopening

[davidsaada]

Ugh... Needs to be reopened again (At least SPIF should include this fix).

[geky]

@davidsaada, just in case you didn't know, the "resolves" keyword in your PRs is what's closing the issue:
https://help.github.com/articles/closing-issues-using-keywords

I don't know if you want this reopened again.

[davidsaada]

just in case you didn't know, the "resolves" keyword in your PRs is what's closing the issue

Thanks, learning something new every day.

[davidsaada]

And don't think I need it reopened. Do you think I need to do that in other external BDs (namely DataFlash and FlashIAP)?

[MarceloSalazar]

@geky can you please confirm @davidsaada question?

[davidsaada]

@MarceloSalazar That won't be necessary. Did that in all left external BDs (and this was reviewed by @geky as well).

[MarceloSalazar]

Thanks for confirming @davidsaada !