[pacman-dev] [PATCH 1/3] diskspace: dedupe code for loading FS usage
add mount_point_load_fsinfo() for platforms using getmntent(). Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- This is just code movement for the following patches. lib/libalpm/diskspace.c | 52 ++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index daee244..f3f9092 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -69,6 +69,23 @@ static void mount_point_list_free(alpm_list_t *mount_points) FREELIST(mount_points); } +static int mount_point_load_fsinfo(alpm_handle_t *handle, alpm_mountpoint_t *mountpoint) +{ +#ifdef HAVE_GETMNTENT + /* grab the filesystem usage */ + if(statvfs(mountpoint->mount_dir, &(mountpoint->fsp)) != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, + _("could not get filesystem information for %s: %s\n"), + mountpoint->mount_dir, strerror(errno)); + return -1; + } + + mountpoint->read_only = mountpoint->fsp.f_flag & ST_RDONLY; +#endif + + return 0; +} + static alpm_list_t *mount_point_list(alpm_handle_t *handle) { alpm_list_t *mount_points = NULL, *ptr; @@ -86,24 +103,14 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) } while((mnt = getmntent(fp))) { - struct statvfs fsp; - if(!mnt) { - _alpm_log(handle, ALPM_LOG_WARNING, - _("could not get filesystem information\n")); - continue; - } - if(statvfs(mnt->mnt_dir, &fsp) != 0) { - _alpm_log(handle, ALPM_LOG_WARNING, - _("could not get filesystem information for %s: %s\n"), - mnt->mnt_dir, strerror(errno)); - continue; - } - CALLOC(mp, 1, sizeof(alpm_mountpoint_t), RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(mnt->mnt_dir); mp->mount_dir_len = strlen(mp->mount_dir); - memcpy(&(mp->fsp), &fsp, sizeof(struct statvfs)); - mp->read_only = fsp.f_flag & ST_RDONLY; + if(mount_point_load_fsinfo(handle, mp) < 0) { + free(mp->mount_dir); + free(mp); + continue; + } mount_points = alpm_list_add(mount_points, mp); } @@ -122,19 +129,14 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) } while((ret = getmntent(fp, &mnt)) == 0) { - struct statvfs fsp; - if(statvfs(mnt->mnt_mountp, &fsp) != 0) { - _alpm_log(handle, ALPM_LOG_WARNING, - _("could not get filesystem information for %s: %s\n"), - mnt->mnt_mountp, strerror(errno)); - continue; - } - CALLOC(mp, 1, sizeof(alpm_mountpoint_t), RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(mnt->mnt_mountp); mp->mount_dir_len = strlen(mp->mount_dir); - memcpy(&(mp->fsp), &fsp, sizeof(struct statvfs)); - mp->read_only = fsp.f_flag & ST_RDONLY; + if(mount_point_load_fsinfo(handle, mp) < 0) { + free(mp->mount_dir); + free(mp); + continue; + } mount_points = alpm_list_add(mount_points, mp); } -- 1.7.10.1
Only load filesystem details for the mount points that we're actually going to write to. This reduces our syscall count considerably. In the case of installation, we would actually stat every mountpoint twice (an extra round for download diskspace) which means (on my system) a total of 60 syscalls to write to 3 partitions when installing the kernel package. This change reduces the 60 syscalls down to the expected 3. A slight debug output change is added here to discern between a mountpoint added to our linked list versus when we actually load the fs info. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- The original impetus for this was to placate automounters (lolsystemd), but even without that, it makes sense to only touch what we're interested in. lib/libalpm/diskspace.c | 49 ++++++++++++++++++++++++++++++++++++----------- lib/libalpm/diskspace.h | 7 +++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index f3f9092..c05de3b 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -77,10 +77,13 @@ static int mount_point_load_fsinfo(alpm_handle_t *handle, alpm_mountpoint_t *mou _alpm_log(handle, ALPM_LOG_WARNING, _("could not get filesystem information for %s: %s\n"), mountpoint->mount_dir, strerror(errno)); + mountpoint->fsinfo_loaded = MOUNT_FSINFO_FAIL; return -1; } + _alpm_log(ALPM_LOG_DEBUG, "loading fsinfo for %s\n", mountpoint->mount_dir); mountpoint->read_only = mountpoint->fsp.f_flag & ST_RDONLY; + mountpoint->fsinfo_loaded = MOUNT_FSINFO_LOADED; #endif return 0; @@ -106,11 +109,6 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) CALLOC(mp, 1, sizeof(alpm_mountpoint_t), RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(mnt->mnt_dir); mp->mount_dir_len = strlen(mp->mount_dir); - if(mount_point_load_fsinfo(handle, mp) < 0) { - free(mp->mount_dir); - free(mp); - continue; - } mount_points = alpm_list_add(mount_points, mp); } @@ -132,11 +130,6 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) CALLOC(mp, 1, sizeof(alpm_mountpoint_t), RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(mnt->mnt_mountp); mp->mount_dir_len = strlen(mp->mount_dir); - if(mount_point_load_fsinfo(handle, mp) < 0) { - free(mp->mount_dir); - free(mp); - continue; - } mount_points = alpm_list_add(mount_points, mp); } @@ -169,6 +162,9 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) mp->read_only = fsp->f_flags & MNT_RDONLY; #endif + /* we don't support lazy loading on this platform */ + mp->usage_loaded = 1; + mount_points = alpm_list_add(mount_points, mp); } #endif @@ -177,7 +173,7 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) mount_point_cmp); for(ptr = mount_points; ptr != NULL; ptr = ptr->next) { mp = ptr->data; - _alpm_log(handle, ALPM_LOG_DEBUG, "mountpoint: %s\n", mp->mount_dir); + _alpm_log(handle, ALPM_LOG_DEBUG, "discovered mountpoint: %s\n", mp->mount_dir); } return mount_points; } @@ -245,6 +241,18 @@ static int calculate_removed_size(alpm_handle_t *handle, continue; } + /* don't check a mount that we know we can't stat */ + if(mp && mp->fsinfo_loaded == MOUNT_FSINFO_FAIL) { + continue; + } + + /* lazy load filesystem info */ + if(mp->fsinfo_loaded == MOUNT_FSINFO_UNLOADED) { + if(mount_point_load_fsinfo(handle, mp) < 0) { + continue; + } + } + /* the addition of (divisor - 1) performs ceil() with integer division */ remove_size = (st.st_size + mp->fsp.f_bsize - 1) / mp->fsp.f_bsize; mp->blocks_needed -= remove_size; @@ -292,6 +300,18 @@ static int calculate_installed_size(alpm_handle_t *handle, continue; } + /* don't check a mount that we know we can't stat */ + if(mp && mp->fsinfo_loaded == MOUNT_FSINFO_FAIL) { + continue; + } + + /* lazy load filesystem info */ + if(mp->fsinfo_loaded == MOUNT_FSINFO_UNLOADED) { + if(mount_point_load_fsinfo(handle, mp) < 0) { + continue; + } + } + /* the addition of (divisor - 1) performs ceil() with integer division */ install_size = (file->size + mp->fsp.f_bsize - 1) / mp->fsp.f_bsize; mp->blocks_needed += install_size; @@ -344,6 +364,13 @@ int _alpm_check_downloadspace(alpm_handle_t *handle, const char *cachedir, goto finish; } + if(cachedir_mp->fsinfo_loaded == MOUNT_FSINFO_UNLOADED) { + if(mount_point_load_fsinfo(handle, cachedir_mp)) { + error = 1; + goto finish; + } + } + /* there's no need to check for a R/O mounted filesystem here, as * _alpm_filecache_setup will never give us a non-writable directory */ diff --git a/lib/libalpm/diskspace.h b/lib/libalpm/diskspace.h index a54aa5e..591d933 100644 --- a/lib/libalpm/diskspace.h +++ b/lib/libalpm/diskspace.h @@ -37,6 +37,12 @@ enum mount_used_level { USED_INSTALL = (1 << 1), }; +enum mount_fsinfo { + MOUNT_FSINFO_UNLOADED = 0, + MOUNT_FSINFO_LOADED, + MOUNT_FSINFO_FAIL, +}; + typedef struct __alpm_mountpoint_t { /* mount point information */ char *mount_dir; @@ -46,6 +52,7 @@ typedef struct __alpm_mountpoint_t { blkcnt_t max_blocks_needed; enum mount_used_level used; int read_only; + enum mount_fsinfo fsinfo_loaded; FSSTATSTYPE fsp; } alpm_mountpoint_t; -- 1.7.10.1
With lazy loading in place, it's now quite obvious that we aren't necessarily checking the right mountpoint for necessary download space. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/diskspace.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index c05de3b..f33261a 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -347,9 +347,17 @@ int _alpm_check_downloadspace(alpm_handle_t *handle, const char *cachedir, { alpm_list_t *mount_points; alpm_mountpoint_t *cachedir_mp; + char resolved_cachedir[PATH_MAX]; size_t j; int error = 0; + /* resolve the cachedir path to ensure we check the right mountpoint. We + * handle failures silently, and continue to use the possibly unresolved + * path. */ + if(realpath(cachedir, resolved_cachedir) != NULL) { + cachedir = resolved_cachedir; + } + mount_points = mount_point_list(handle); if(mount_points == NULL) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not determine filesystem mount points\n")); -- 1.7.10.1
participants (1)
-
Dave Reisner