Commit cc2f95a6 authored by Marvin Scholz's avatar Marvin Scholz

mmc_device_darwin: Rewrite mounting logic

The previous way of mounting the disk was flawed which especially
showed after moving to using a dispatch queue that removed long wait
times.

When the exclusive low level access is released, the OS will see the
disk as if it is a "new" device and then try to mount it again, so it
does not work to immediately try to mount the disk again as it would
fail. The disk is not mounted for us, regardless of the failure, thats
done because the OS mounts the disk. Even removing the mount call
completely would still result in the disk getting mounted by the OS,
so calling iokit_mount was unnecessary.

Yet we want to mount the disk ourselves so we know when it is usable
and completely mounted for reading files on it. To do that my approach
is to use a mount approval callback, this callback gets called whenever
a device is to be mounted by someone (usually the OS auto-mounting) and
then the callback can decide to allow or deny mounting.

In this callback we check if the disk is currently not mounted, then we
compare the disk with our disk to know if they match.
Internally what that does is just compare the BSD name so it is not
entirely perfect as the user could unplug the disk and plug a different
one and it could appear as the "same" disk.

If the disks match, we reject the mount attempt and indicate to the
caller that the disk appeared, using the disk_state.

The caller uses a semaphore to wait for a given timeout, currently
20 seconds. Waiting indefinitely is impossible as it could cause
the program to hang if the user unplugs the drive while we wait.

When the disk re-appears we now mount it, during that the state has to
be set to mounting so that the mount approval callback can allow the
request when we try to mount. Just unregistering the mount approval
callback after our disk appeared the first time does not work as
it causes the OS to immediately re-try mounting the disk apparently.

Once the mounting succeeded we can proceed. Even if it fails though,
there is currently not much that can be done about that, so the return
value of the iokit_mount is ignored.
parent d62f878f
......@@ -70,6 +70,12 @@
/*
*
*/
enum disk_state_e {
disk_mounted,
disk_unmounted,
disk_appeared,
disk_mounting
};
struct mmcdev {
/* Interfaces required for low-level device communication */
......@@ -83,8 +89,9 @@ struct mmcdev {
/* for mounting/unmounting the disc */
DADiskRef disk;
DASessionRef session;
bool is_mounted;
enum disk_state_e disk_state;
dispatch_semaphore_t sync_sem;
dispatch_queue_t background_queue;
};
int device_send_cmd(MMCDEV *mmc, const uint8_t *cmd, uint8_t *buf, size_t tx, size_t rx)
......@@ -193,7 +200,7 @@ static void iokit_unmount_complete (DADiskRef disk, DADissenterRef dissenter,
BD_DEBUG(DBG_MMC, "Could not unmount the disc\n");
} else {
BD_DEBUG(DBG_MMC, "Disc unmounted\n");
((MMCDEV *)context)->is_mounted = 0;
((MMCDEV *)context)->disk_state = disk_unmounted;
}
dispatch_semaphore_signal(((MMCDEV *)context)->sync_sem);
}
......@@ -201,28 +208,24 @@ static void iokit_unmount_complete (DADiskRef disk, DADissenterRef dissenter,
static void iokit_mount_complete (DADiskRef disk, DADissenterRef dissenter,
void *context) {
(void) disk; /* suppress warning */
(void) dissenter; /* suppress warning */
if (dissenter) {
BD_DEBUG(DBG_MMC, "Could not mount the disc\n");
DAReturn code = DADissenterGetStatus(dissenter);
BD_DEBUG(DBG_MMC, "Could not mount the disc (%8X)\n", code);
((MMCDEV *)context)->disk_state = disk_unmounted;
} else {
BD_DEBUG(DBG_MMC, "Disc mounted\n");
((MMCDEV *)context)->disk_state = disk_mounted;
}
/* FIXME: The disc does not actually mount whether there is
* a dissenter or not, the OS mounts the disc automatically
* kind of racing against us mounting the disc.
* It is pure luck if the disc is mounted or not, sometimes
* we are lucky enough, especially because the runloop is
* running for 10 seconds, which most of the time is long
* enough for the OS to mount the disc again.
*/
((MMCDEV *)context)->is_mounted = 1;
dispatch_semaphore_signal(((MMCDEV *)context)->sync_sem);
}
/* Unmount the disk at mmc->disk
* Note: This MAY NOT be called on the background queue,
* as that would lead to a deadlock.
*/
static int iokit_unmount (MMCDEV *mmc) {
if (0 == mmc->is_mounted) {
if (disk_unmounted == mmc->disk_state) {
return 0; /* nothing to do */
}
......@@ -231,18 +234,22 @@ static int iokit_unmount (MMCDEV *mmc) {
DADiskUnmount (mmc->disk, kDADiskUnmountOptionForce, iokit_unmount_complete, mmc);
dispatch_semaphore_wait (mmc->sync_sem, DISPATCH_TIME_FOREVER);
return mmc->is_mounted ? -1 : 0;
return (mmc->disk_state == disk_unmounted) ? 0 : -1;
}
/* Mount the disk at mmc->disk
* Note: This MAY NOT be called on the background queue,
* as that would lead to a deadlock.
*/
static int iokit_mount (MMCDEV *mmc) {
if (0 == mmc->is_mounted) {
if (disk_mounted != mmc->disk_state) {
if (mmc->disk && mmc->session) {
mmc->disk_state = disk_mounting;
DADiskMount (mmc->disk, NULL, kDADiskMountOptionDefault, iokit_mount_complete, mmc);
dispatch_semaphore_wait(mmc->sync_sem, DISPATCH_TIME_FOREVER);
dispatch_semaphore_wait (mmc->sync_sem, DISPATCH_TIME_FOREVER);
}
}
return mmc->is_mounted ? 0 : -1;
return (mmc->disk_state == disk_unmounted) ? 0 : -1;
}
static int iokit_find_service_matching (MMCDEV *mmc, io_service_t *servp) {
......@@ -326,6 +333,39 @@ static int iokit_find_interfaces (MMCDEV *mmc, io_service_t service) {
return 0;
}
static DADissenterRef iokit_mount_approval_cb(DADiskRef disk, void *context)
{
MMCDEV *mmc = context;
/* If the disk state is mounted, there is nothing to do here. */
if (disk_mounted == mmc->disk_state) {
return NULL;
}
/* Check if the disk that is to be mounted matches ours,
* if not, we do not need to reject mounting.
*/
if (!CFEqual(disk, mmc->disk)) {
return NULL;
}
BD_DEBUG(DBG_MMC, "Mount approval request for matching disc\n");
/* When we are trying to mount, the mount approval callback is
* called too, so we need to allow mounting for ourselves here.
*/
if (disk_mounting == mmc->disk_state) {
BD_DEBUG(DBG_MMC, "Allowing ourselves to mount\n");
return NULL;
}
mmc->disk_state = disk_appeared;
dispatch_semaphore_signal(mmc->sync_sem);
CFStringRef reason = CFSTR("Disk is going to be mounted libaacs");
return DADissenterCreate(kCFAllocatorDefault, kDAReturnBusy, reason);
}
static int iokit_da_init(MMCDEV *mmc) {
mmc->session = DASessionCreate (kCFAllocatorDefault);
if (NULL == mmc->session) {
......@@ -341,8 +381,11 @@ static int iokit_da_init(MMCDEV *mmc) {
return -1;
}
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
DASessionSetDispatchQueue(mmc->session, queue);
mmc->background_queue = dispatch_queue_create("org.videolan.libaacs", DISPATCH_QUEUE_SERIAL);
DASessionSetDispatchQueue(mmc->session, mmc->background_queue);
// Register event callbacks
DARegisterDiskMountApprovalCallback(mmc->session, NULL, iokit_mount_approval_cb, mmc);
mmc->sync_sem = dispatch_semaphore_create(0);
......@@ -350,18 +393,23 @@ static int iokit_da_init(MMCDEV *mmc) {
}
static void iokit_da_destroy(MMCDEV *mmc) {
DASessionSetDispatchQueue(mmc->session, NULL);
if (mmc->session) {
/* The approval callback must be unregistered here, doing it in the
* mount approval callback instead after we got a matching disk would
* cause the OS to immediately re-try to mount the disk faster than we
* can mount it.
*/
DAUnregisterApprovalCallback(mmc->session, iokit_mount_approval_cb, mmc);
DASessionSetDispatchQueue(mmc->session, NULL);
CFRelease (mmc->session);
mmc->session = NULL;
}
if (mmc->disk) {
CFRelease (mmc->disk);
mmc->disk = NULL;
}
if (mmc->session) {
CFRelease (mmc->session);
mmc->session = NULL;
}
dispatch_release(mmc->sync_sem);
}
......@@ -374,7 +422,7 @@ static int mmc_open_iokit (const char *path, MMCDEV *mmc) {
mmc->taskInterface = NULL;
mmc->disk = NULL;
mmc->session = NULL;
mmc->is_mounted = true;
mmc->disk_state = disk_mounted;
/* get the bsd name associated with this mount */
rc = get_mounted_device_from_path (mmc, path);
......@@ -444,9 +492,30 @@ MMCDEV *device_open(const char *path)
void device_close(MMCDEV **pp)
{
__block int rc = 0;
if (pp && *pp) {
MMCDEV *mmc = *pp;
/* When the exclusive access to the drive is released,
* the OS will see the device like a "new" device and
* try to mount it. Therefore we can't just mount the
* disk we previously got immediately here as it would
* fail with kDAReturnBadArgument as the disk is not
* available yet.
* Trying to mount the disk after it appears in peek
* does not work either as the disk is not yet ready
* or in the process of being mounted by the OS so
* that would return an kDAReturnBusy error.
* The only way that seems to reliably work is to use
* a mount approval callback. When the OS tries to
* mount the disk, the mount approval callback is
* called and we can reject mounting and then proceed
* to mount the disk ourselves.
* Claiming exclusive access using DADiskClaim in order
* to prevent the OS form mounting the disk does not work
* either!
*/
if (mmc->taskInterface) {
(*mmc->taskInterface)->ReleaseExclusiveAccess (mmc->taskInterface);
(*mmc->taskInterface)->Release (mmc->taskInterface);
......@@ -462,10 +531,39 @@ void device_close(MMCDEV **pp)
IODestroyPlugInInterface(mmc->plugInInterface);
}
(void) iokit_mount (mmc);
iokit_da_destroy(mmc);
/* Wait for disc to re-appear for 20 seconds.
* This timeout was figured out by experimentation with
* a USB BD drive which sometimes can take really long to
* be in a mountable state again.
* For internal drives this is probably much faster
* so the long timeout shouldnt do much harm for thse
* cases.
*/
dispatch_time_t timeout = dispatch_time(DISPATCH_TIME_NOW, 20 * 1E+9);
dispatch_semaphore_wait(mmc->sync_sem, timeout);
/* It is crucial that this is done on the event handling queue
* else callbacks could be received while this code runs.
*/
dispatch_sync(mmc->background_queue, ^{
if (disk_appeared != mmc->disk_state) {
BD_DEBUG(DBG_MMC | DBG_CRIT, "Timeout waiting for the disc to appear again!\n");
iokit_da_destroy(mmc);
rc = -1;
return;
}
rc = 0;
});
if (rc == 0) {
/* Disk appeared successfully, mount it.
* Return value is ignored as logging of success or
* error takes place in the callback already and there
* is nothing we can do really if mounting fails.
*/
(void) iokit_mount(mmc);
iokit_da_destroy(mmc);
}
X_FREE(*pp);
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment