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.

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

Changes to support mmap reads on Broker #57

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions src/modules/dyad.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,10 @@ dyad_fetch_request_cb (flux_t *h, flux_msg_handler_t *w, const flux_msg_t *msg,
#endif // DYAD_SPIN_WAIT

FLUX_LOG_INFO (h, "Reading file %s for transfer", fullpath);
fd = open (fullpath, O_RDONLY);
if (fd < 0) {
FLUX_LOG_ERR (h, "DYAD_MOD: Failed to open file \"%s\".\n", fullpath);
goto fetch_error;
}
if ((inlen = read_all (fd, &inbuf)) < 0) {
if ((inlen = read_all (fullpath, &inbuf)) < 0) {
FLUX_LOG_ERR (h, "DYAD_MOD: Failed to load file \"%s\".\n", fullpath);
goto fetch_error;
}
close (fd);
FLUX_LOG_INFO (h, "Is inbuf NULL? -> %i\n", (int)(inbuf == NULL));

FLUX_LOG_INFO (h, "Establish DTL connection with consumer");
Expand All @@ -188,7 +182,7 @@ dyad_fetch_request_cb (flux_t *h, flux_msg_handler_t *w, const flux_msg_t *msg,
errno = ECOMM;
goto fetch_error;
}

munmap(inbuf, inlen);
FLUX_LOG_INFO (h, "Close RPC message stream with an ENODATA (%d) message", ENODATA);
if (flux_respond_error (h, msg, ENODATA, NULL) < 0) {
FLUX_LOG_ERR (h,
Expand Down
23 changes: 15 additions & 8 deletions src/utils/read_all.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@
#if HAVE_CONFIG_H
#include "config.h"
#endif // HAVE_CONFIG_H

#define _GNU_SOURCE
#include <errno.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>

#include <fcntl.h>
#include <sys/mman.h>
#include "read_all.h"

ssize_t write_all (int fd, const void *buf, size_t len)
Expand All @@ -37,8 +40,13 @@ ssize_t write_all (int fd, const void *buf, size_t len)
return count;
}

ssize_t read_all (int fd, void **bufp)
ssize_t read_all (const char* filename, void **bufp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface probably should take a page aligned memory address as an input for the UCX managed memory. So, the allocation would happen outside of this function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the buffer bufp. Once we do the UCX, we can move allocation there.

{
int fd = open (filename, O_RDONLY);
if (fd < 0) {
errno = EINVAL;
return -1;
}
const ssize_t file_size = lseek (fd, 0, SEEK_END);
if (file_size == 0) {
errno = EINVAL;
Expand All @@ -49,16 +57,15 @@ ssize_t read_all (int fd, void **bufp)
errno = EINVAL;
return -1;
}
*bufp = malloc (file_size);
size_t alignment = getpagesize();
*bufp = aligned_alloc(alignment, file_size);
if (*bufp == NULL) {
errno = EINVAL;
return -1;
}
ssize_t bytes_read = read (fd, *bufp, file_size);
if (bytes_read < file_size) {
// could not read all data
if ((*bufp = mmap (*bufp, file_size, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, 0)) == (caddr_t) -1) {
errno = EINVAL;
return bytes_read;
return -1;
}
return file_size;
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/read_all.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ ssize_t write_all (int fd, const void *buf, size_t len);
__attribute__ ((annotate ("@critical_path()")))
#endif
ssize_t
read_all (int fd, void **bufp);
read_all (const char* filename, void **bufp);

#if defined(__cplusplus)
};
Expand Down