Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Block hotplug is accidentally quadratic #2

Open
nwf opened this issue May 20, 2023 · 2 comments · May be fixed by #3
Open

Block hotplug is accidentally quadratic #2

nwf opened this issue May 20, 2023 · 2 comments · May be fixed by #3

Comments

@nwf
Copy link

nwf commented May 20, 2023

The cache_load call in mount_action ignores the device name parameter:

fstools/block.c

Line 1256 in bfe882d

cache_load(1);

As such, each hotplug event (specifically, each /sbin/block hotplug invoked via /etc/hotplug.d/block/10-mount via /sbin/hotplug-call through the machinations of /etc/hotplug.json) probes all the block devices named here:

fstools/block.c

Lines 580 to 596 in bfe882d

static void cache_load(int mtd)
{
if (mtd) {
_cache_load("/dev/mtdblock*");
_cache_load("/dev/ubiblock*");
_cache_load("/dev/ubi[0-9]*");
}
_cache_load("/dev/loop*");
_cache_load("/dev/mmcblk*");
_cache_load("/dev/sd*");
_cache_load("/dev/hd*");
_cache_load("/dev/md*");
_cache_load("/dev/nvme*");
_cache_load("/dev/vd*");
_cache_load("/dev/xvd*");
_cache_load("/dev/dm-*");
}

That's rather a lot, if not all, of the block devices on the system, making the total block hotplug sequence accidentally quadratic.

I hope this is the right place to raise this issue, and I've also documented a bit about how I found this over in this forum thread, but if it belongs somewhere else, please let me know.
nwf added a commit to nwf/fstools that referenced this issue Jun 17, 2023
@nwf
mount_action: populate just one device in cache
2578681 
We're given the device basename as a parameter and then search through
the cache to find it.  Just load the one device, saving the probes for
the ones we don't care about.

FIXES openwrt#2
@nwf nwf linked a pull request Jun 17, 2023 that will close this issue
Open
nwf added a commit to nwf/fstools that referenced this issue Jun 17, 2023
@nwf
mount_action: populate just one device in cache
914e0a6 
We're given the device basename as a parameter and then search through
the cache to find it.  Just load the one device, saving the probes for
the ones we don't care about.

FIXES openwrt#2
nwf added a commit to nwf/fstools that referenced this issue Jun 24, 2023
@nwf
mount_action: populate just one device in cache
0c49b7f 
We're given the device basename as a parameter and then search through
the cache to find it.  Just load the one device, saving the probes for
the ones we don't care about.

FIXES openwrt#2
@eric-j-ason
Copy link

In addition to your optimization, isn't it the case that one could break the loop just after cache_load, once a match has been found, as there should only ever be one match, right?

fstools/block.c

Lines 1258 to 1260 in bfe882d

list_for_each_entry(pr, &devices, list)
if (!strcmp(basename(pr->dev), device))
path = pr->dev;

@nwf
Copy link
Author

nwf commented Apr 11, 2024

Apologies for dropping the ball. Yes, I suspect it is safe to break out of that loop as you describe. Should I amend the PR?

nwf added a commit to nwf/fstools that referenced this issue Jun 2, 2024
@nwf
mount_action: populate just one device in cache
a3f2c15 
We're given the device basename as a parameter and then search through
the cache to find it.  Just load the one device, saving the probes for
the ones we don't care about.

FIXES openwrt#2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

mount_action: populate just one device in cache nwf/fstools
2 participants
@nwf @eric-j-ason