From 0eb6c8ee24289cd8ea395911e5f0fd265fe1a43f Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 5 Nov 2019 17:05:50 -0500 Subject: [PATCH] src.ctf.fs: factor out "ds_file_mmap" from "ds_file_mmap_next" Currently, the logic to mmap a region of a ds_file is contained in ds_file_mmap_next. ds_file_mmap_next is also responsible for mapping the region immediately following the currently mapped region, if there is a currently mapped region. I find this double responsibility a bit confusing, especially where ds_file_mmap_next is used in medop_seek. This patch extracts a portion of ds_file_mmap_next to a new function ds_file_mmap, whose job is to ensure a file offset is mapped, and to do the needful if it isn't. Change-Id: Ia19725d28cf9c24084a52d16c7a88f30b32ff694 Signed-off-by: Simon Marchi --- src/plugins/ctf/fs-src/data-stream-file.c | 159 +++++++++++++--------- 1 file changed, 93 insertions(+), 66 deletions(-) diff --git a/src/plugins/ctf/fs-src/data-stream-file.c b/src/plugins/ctf/fs-src/data-stream-file.c index 078e8b5f..7fe4e6a5 100644 --- a/src/plugins/ctf/fs-src/data-stream-file.c +++ b/src/plugins/ctf/fs-src/data-stream-file.c @@ -51,6 +51,17 @@ size_t remaining_mmap_bytes(struct ctf_fs_ds_file *ds_file) return ds_file->mmap_len - ds_file->request_offset_in_mapping; } +/* + * Return true if `offset_in_file` is in the current mapping. + */ + +static +bool offset_ist_mapped(struct ctf_fs_ds_file *ds_file, off_t offset_in_file) +{ + return offset_in_file >= ds_file->mmap_offset_in_file && + offset_in_file < (ds_file->mmap_offset_in_file + ds_file->mmap_len); +} + static enum ctf_msg_iter_medium_status ds_file_munmap( struct ctf_fs_ds_file *ds_file) @@ -83,38 +94,57 @@ end: return status; } +/* + * mmap a region of `ds_file` such that `requested_offset_in_file` is in the + * mapping. If the currently mmap-ed region already contains + * `requested_offset_in_file`, the mapping is kept. + * + * Set `ds_file->requested_offset_in_mapping` based on `request_offset_in_file` + * + * `requested_offset_in_file` must be a valid offset in the file. + */ static -enum ctf_msg_iter_medium_status ds_file_mmap_next( - struct ctf_fs_ds_file *ds_file) +enum ctf_msg_iter_medium_status ds_file_mmap( + struct ctf_fs_ds_file *ds_file, off_t requested_offset_in_file) { enum ctf_msg_iter_medium_status status; bt_self_component *self_comp = ds_file->self_comp; bt_logging_level log_level = ds_file->log_level; - /* Unmap old region */ - if (ds_file->mmap_addr) { - status = ds_file_munmap(ds_file); - if (status != CTF_MSG_ITER_MEDIUM_STATUS_OK) { - goto end; - } + /* Ensure the requested offset is in the file range. */ + BT_ASSERT(requested_offset_in_file >= 0); + BT_ASSERT(requested_offset_in_file < ds_file->file->size); - /* - * mmap_len is guaranteed to be page-aligned except on the - * last mapping where it may not be possible (since the file's - * size itself may not be a page multiple). - */ - ds_file->mmap_offset_in_file += ds_file->mmap_len; - ds_file->request_offset_in_mapping = 0; + /* + * If the mapping already contains the requested offset, just adjust + * requested_offset_in_mapping. + */ + if (offset_ist_mapped(ds_file, requested_offset_in_file)) { + ds_file->request_offset_in_mapping = + requested_offset_in_file - ds_file->mmap_offset_in_file; + status = CTF_MSG_ITER_MEDIUM_STATUS_OK; + goto end; } - ds_file->mmap_len = MIN(ds_file->file->size - ds_file->mmap_offset_in_file, - ds_file->mmap_max_len); - if (ds_file->mmap_len == 0) { - status = CTF_MSG_ITER_MEDIUM_STATUS_EOF; + /* Unmap old region */ + status = ds_file_munmap(ds_file); + if (status != CTF_MSG_ITER_MEDIUM_STATUS_OK) { goto end; } - /* Map new region */ - BT_ASSERT(ds_file->mmap_len); + + /* + * Compute a mapping that has the required alignment properties and + * contains `requested_offset_in_file`. + */ + ds_file->request_offset_in_mapping = + requested_offset_in_file % bt_mmap_get_offset_align_size(ds_file->log_level); + ds_file->mmap_offset_in_file = + requested_offset_in_file - ds_file->request_offset_in_mapping; + ds_file->mmap_len = MIN(ds_file->file->size - ds_file->mmap_offset_in_file, + ds_file->mmap_max_len); + + BT_ASSERT(ds_file->mmap_len > 0); + ds_file->mmap_addr = bt_mmap((void *) 0, ds_file->mmap_len, PROT_READ, MAP_PRIVATE, fileno(ds_file->file->fp), ds_file->mmap_offset_in_file, ds_file->log_level); @@ -128,6 +158,46 @@ enum ctf_msg_iter_medium_status ds_file_mmap_next( } status = CTF_MSG_ITER_MEDIUM_STATUS_OK; + +end: + return status; +} + +/* + * Change the mapping of the file to read the region that follows the current + * mapping. + * + * If the file hasn't been mapped yet, then everything (mmap_offset_in_file, + * mmap_len, request_offset_in_mapping) should have the value 0, which will + * result in the beginning of the file getting mapped. + * + * return _EOF if the current mapping is the end of the file. + */ + +static +enum ctf_msg_iter_medium_status ds_file_mmap_next( + struct ctf_fs_ds_file *ds_file) +{ + enum ctf_msg_iter_medium_status status; + + /* + * If we're called, it's because more bytes are requested but we have + * given all the bytes of the current mapping. + */ + BT_ASSERT(ds_file->request_offset_in_mapping == ds_file->mmap_len); + + /* + * If the current mapping coincides with the end of the file, there is + * no next mapping. + */ + if (ds_file->mmap_offset_in_file + ds_file->mmap_len == ds_file->file->size) { + status = CTF_MSG_ITER_MEDIUM_STATUS_EOF; + goto end; + } + + status = ds_file_mmap(ds_file, + ds_file->mmap_offset_in_file + ds_file->mmap_len); + end: return status; } @@ -216,55 +286,12 @@ end: static enum ctf_msg_iter_medium_status medop_seek(off_t offset, void *data) { - enum ctf_msg_iter_medium_status status; struct ctf_fs_ds_file *ds_file = data; - off_t offset_in_mapping, file_size = ds_file->file->size; - bt_self_component *self_comp = ds_file->self_comp; - bt_logging_level log_level = ds_file->log_level; BT_ASSERT(offset >= 0); - BT_ASSERT(offset < file_size); + BT_ASSERT(offset < ds_file->file->size); - /* If there is no current mapping, map the right file directly. */ - if (!ds_file->mmap_addr) { - goto map_requested_offset; - } - - /* - * Determine whether or not the destination is contained within the - * current mapping. - */ - if (offset < ds_file->mmap_offset_in_file || - offset >= ds_file->mmap_offset_in_file + ds_file->mmap_len) { - BT_COMP_LOGD("Medium seek request cannot be accomodated by the current " - "file mapping: offset=%jd, mmap-offset=%jd, " - "mmap-len=%zu", (intmax_t) offset, (intmax_t) ds_file->mmap_offset_in_file, - ds_file->mmap_len); - status = ds_file_munmap(ds_file); - if (status != CTF_MSG_ITER_MEDIUM_STATUS_OK) { - goto end; - } - goto map_requested_offset; - } else { - ds_file->request_offset_in_mapping = offset - ds_file->mmap_offset_in_file; - status = CTF_MSG_ITER_MEDIUM_STATUS_OK; - goto end; - } - -map_requested_offset: - offset_in_mapping = offset % - bt_mmap_get_offset_align_size(ds_file->log_level); - - ds_file->mmap_offset_in_file = offset - offset_in_mapping; - ds_file->request_offset_in_mapping = offset_in_mapping; - status = ds_file_mmap_next(ds_file); - if (status != CTF_MSG_ITER_MEDIUM_STATUS_OK) { - goto end; - } - - status = CTF_MSG_ITER_MEDIUM_STATUS_OK; -end: - return status; + return ds_file_mmap(ds_file, offset); } BT_HIDDEN -- 2.34.1