diff --git a/README.md b/README.md index eca8768..cb58006 100644 --- a/README.md +++ b/README.md @@ -240,10 +240,13 @@ report=none ### Partition Nodes -Whenever a node that answers `DMDRVI_IOCTL_BLOCK_GET_INFO` (and whose -`report=` allows `block`) becomes available - at mount, when a pending configuration becomes ready, or when a +Whenever a node that answers `DMDRVI_IOCTL_BLOCK_GET_INFO` becomes +available - at mount, when a pending configuration becomes ready, or when a driver announces it with `dmdrvi_device_available()` - DMDEVFS reads its -partition table and creates one node per partition next to it: +partition table and creates one node per partition next to it. Only nodes +already reported as `block` (see above) are read, so a node whose +`report=` excludes `block` gets no partition nodes, and no other node is +opened for it: ``` /dev/dmsdio0/0 whole medium diff --git a/src/dmdevfs.c b/src/dmdevfs.c index 0a9c465..25a80e5 100644 --- a/src/dmdevfs.c +++ b/src/dmdevfs.c @@ -3254,15 +3254,26 @@ static void publish_partition( dmfsi_context_t mount, const char* mount_path, dr * as "block" and withdrawn together with their parent * (remove_partitions_of()). * + * Always runs after notify_driver_path_ready(), whose report_node_added() + * has already probed the node: only a node reported as "block" is opened + * again to read its table. Every other node (tty, gpio, dma, ...) is left + * alone - a second open of each would be wasted, and fails loudly for + * drivers that open exclusively (e.g. a dmdma stream leased by another + * driver, or telnetd, which hangs up a connection when its node is + * closed). A node whose report= excludes "block" gets no partition nodes. + * * Must be called without g_devfs_mutex held (driver code). */ static void scan_partitions( dmfsi_context_t mount, const char* mount_path, driver_node_t* node ) { - // Only nodes that may be block devices (report= mask) - probing opens and - // closes the node, which for some drivers is not free of side effects - // (telnetd hangs up the connection when its node is closed) - if (node == NULL || node->is_builtin || node->is_partition || - (node->report_mask & DMDEVFS_REPORT_BLOCK) == 0) + if (node == NULL || node->is_builtin || node->is_partition) + { + return; + } + dmosi_mutex_lock(g_devfs_mutex); + bool is_block = (node->reported & DMDEVFS_REPORT_BLOCK) != 0; + dmosi_mutex_unlock(g_devfs_mutex); + if (!is_block) { return; } diff --git a/tests/mockdrv/dmdevfs_mockdrv.h b/tests/mockdrv/dmdevfs_mockdrv.h index 1ea0efc..08a3206 100644 --- a/tests/mockdrv/dmdevfs_mockdrv.h +++ b/tests/mockdrv/dmdevfs_mockdrv.h @@ -17,12 +17,14 @@ #include -/** Monitor contract calls a monitored host node received so far. */ +/** Monitor contract calls a monitored host node received so far, and opens of its nodes. */ typedef struct { - uint32_t events; /**< DMDRVI_IOCTL_MONITOR_EVENT */ - uint32_t refreshes; /**< DMDRVI_IOCTL_MONITOR_REFRESH */ - uint32_t create_seq; /**< 1-based order in which dmdevfs created this context */ + uint32_t events; /**< DMDRVI_IOCTL_MONITOR_EVENT */ + uint32_t refreshes; /**< DMDRVI_IOCTL_MONITOR_REFRESH */ + uint32_t create_seq; /**< 1-based order in which dmdevfs created this context */ + uint32_t host_opens; /**< Successful opens of the host node, including the GET_STATS caller's own */ + uint32_t child_opens; /**< Successful opens of the block child */ } dmdevfs_mockdrv_stats_t; /** One raw sector for DMDEVFS_MOCKDRV_IOCTL_WRITE_SECTOR. */ diff --git a/tests/mockdrv/mockdrv.c b/tests/mockdrv/mockdrv.c index 980d5cb..aa8ecdc 100644 --- a/tests/mockdrv/mockdrv.c +++ b/tests/mockdrv/mockdrv.c @@ -13,7 +13,8 @@ * Configuration keys: `major` (device number) and `monitor` ("true": the host * node implements the monitor contract). Its policy comes from * `event_handler`, `settle_ms` and `poll_interval_ms`; MONITOR_EVENT and - * MONITOR_REFRESH calls are only counted (DMDEVFS_MOCKDRV_IOCTL_GET_STATS). + * MONITOR_REFRESH calls are only counted (DMDEVFS_MOCKDRV_IOCTL_GET_STATS), + * as are the opens of the host node and of its child. * The hot-plugged child (minor 0) is a block device on a sparse medium of * DMDEVFS_MOCKDRV_BLOCK_SIZE blocks (default 8, DMDEVFS_MOCKDRV_IOCTL_SET_MEDIA): * only sectors ever written are stored, so 64-bit sized media cost nothing. @@ -37,6 +38,8 @@ struct dmdrvi_context volatile uint32_t events; /* MONITOR_EVENT calls */ volatile uint32_t refreshes; /* MONITOR_REFRESH calls */ uint32_t create_seq; /* Value of g_create_count after this _create */ + volatile uint32_t host_opens; /* Successful _open() of the host node */ + volatile uint32_t child_opens;/* Successful _open() of the child */ uint64_t block_count;/* Medium size of the child */ dmlist_context_t* sectors; /* dmdevfs_mockdrv_sector_t* written so far */ }; @@ -210,6 +213,8 @@ dmod_dmdrvi_dif_api_declaration(2.0, dmdevfs_mockdrv, dmdrvi_context_t, _create, ctx->events = 0; ctx->refreshes = 0; ctx->create_seq = ++g_create_count; + ctx->host_opens = 0; + ctx->child_opens = 0; ctx->block_count = 8; ctx->sectors = dmlist_create(); memset(&ctx->policy, 0, sizeof(ctx->policy)); @@ -249,6 +254,14 @@ dmod_dmdrvi_dif_api_declaration(2.0, dmdevfs_mockdrv, void*, _open, { handle->magic = MOCKDRV_HANDLE_MAGIC; handle->is_child = is_child; + if (is_child) + { + context->child_opens++; + } + else + { + context->host_opens++; + } } return handle; } @@ -358,6 +371,8 @@ static int host_ioctl(dmdrvi_context_t context, int command, void* arg) stats->events = context->events; stats->refreshes = context->refreshes; stats->create_seq = context->create_seq; + stats->host_opens = context->host_opens; + stats->child_opens = context->child_opens; return 0; } case DMDEVFS_MOCKDRV_IOCTL_PLUG: diff --git a/tests/test_dmdevfs.c b/tests/test_dmdevfs.c index 19aee61..8f33771 100644 --- a/tests/test_dmdevfs.c +++ b/tests/test_dmdevfs.c @@ -143,6 +143,19 @@ static int set_plugged(const char* host, bool plugged) return ret; } +/** Driver statistics of /dev/dmdevfs_mockdrv, fetched through dmdevfs (one host open). */ +static int get_stats(const char* host, dmdevfs_mockdrv_stats_t* stats) +{ + void* file = NULL; + if (g_fs.fopen(g_mount, &file, host, DMFSI_O_RDONLY, 0) != DMFSI_OK) + { + return -ENOENT; + } + int ret = g_fs.ioctl(g_mount, file, DMDEVFS_MOCKDRV_IOCTL_GET_STATS, stats); + g_fs.fclose(g_mount, file); + return ret; +} + DMOD_TEST_STEP(dmdevfs_reports_monitored_node_and_its_removal) { DMOD_TEST_EXPECT_NOT_NULL(g_mount); @@ -182,6 +195,27 @@ DMOD_TEST_STEP(dmdevfs_hot_plugged_node_inherits_report_setting) DMOD_TEST_EXPECT_EQ(unit_state("blk@dmdevfs_mockdrv1_0"), -1); } +DMOD_TEST_STEP(dmdevfs_opens_only_block_nodes_to_scan_partitions) +{ + DMOD_TEST_EXPECT_NOT_NULL(g_mount); + DMOD_TEST_EXPECT_EQ(set_plugged("/dmdevfs_mockdrv1", true), 0); /* report=none */ + DMOD_TEST_EXPECT_EQ(set_plugged("/dmdevfs_mockdrv2", true), 0); /* default: all */ + /* One hot-plug thread, in order: mock1's child has been handled once + * mock2's is reported. */ + DMOD_TEST_EXPECT_TRUE(wait_ready("blk@dmdevfs_mockdrv2_0", "/dev/dmdevfs_mockdrv2/0")); + + dmdevfs_mockdrv_stats_t stats; + /* Plain host node: probed once at mount, never opened again for its + * partition table since it is no block device - plus get_stats() and + * set_plugged() themselves. */ + DMOD_TEST_EXPECT_EQ(get_stats("/dmdevfs_mockdrv2", &stats), 0); + DMOD_TEST_EXPECT_EQ(stats.host_opens, 3u); + /* report=none: neither the host nor its block child are opened by dmdevfs. */ + DMOD_TEST_EXPECT_EQ(get_stats("/dmdevfs_mockdrv1", &stats), 0); + DMOD_TEST_EXPECT_EQ(stats.host_opens, 2u); + DMOD_TEST_EXPECT_EQ(stats.child_opens, 0u); +} + DMOD_TEST_STEP(dmdevfs_reports_removal_of_plugged_node_on_teardown) { DMOD_TEST_EXPECT_NOT_NULL(g_mount);