On Sun, Jan 26, 2014 at 03:06:50PM +0000, Rodrigo Campos wrote:
> bash-completion/fallocate | 2 +-
> sys-utils/fallocate.1 | 19 +++++++-
> sys-utils/fallocate.c | 114 ++++++++++++++++++++++++++++++++++++++++------
> 3 files changed, 120 insertions(+), 15 deletions(-)
Applied with some changes and I believe that the code still need some
improvements, see below
> + */
> +static int detect_holes(int fd, size_t hole_size)
> +{
> + int ret = 0;
> + int err;
> +
> + if (hole_size >= 100 * 1024 * 1024) {
> + size_t ram_mb = hole_size / 1024 / 1024;
> + printf("WARNING: %zu MB RAM will be used\n", ram_mb);
> + sleep(3);
> + }
I have removed this thing...
I don't like all the detection algorithm. Do we really need to allocate
all hole size and the buffer?
IMHO it would be enough to:
* add posix_fadvise(... POSIX_FADV_SEQUENTIAL | POSIX_FADV_NOREUSE)
* read the file in small chunks -- for example BUFSIZ and compare
this with small empty static buffer.
.. it's kernel business to read from FS/device in optimal way and I
don't think that context switches are so critical issue when all the
thing is about I/O.
I didn't test it, so maybe I'm wrong, but the current code where we
eat RAM seems too crazy. Comments?
> + /* Create a buffer of '\0's to compare against */
> + /* XXX: Use mmap() with MAP_PRIVATE so Linux can avoid this allocation */
> + void *zeros = mmap(NULL, hole_size, PROT_READ,
> + MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
> + if (zeros == MAP_FAILED) {
> + perror("mmap");
we use err.h stuff, and it's usually good enough to exit after error
than waste time with memory deallocation and another clean ups. The
kernel is smart enough to clean up after exit().
> + off_t end = lseek(fd, 0, SEEK_END);
^^^^^^^^^^^^^^
> + if (end == -1) {
> + perror("lseek");
> + ret = -1;
> + goto out;
> + }
> +
> + for (off_t offset = 0; offset + hole_size <= end; offset += buf_len) {
^^^^^^^^^^^
Yes it's expected by C standards, but it sucks. Don't use it, it's
reader's nightmare.
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
