Re: Recent changes (master)

Andrey Kuzmin
2014-10-08T13:52:41Z

Recent libaio fix could be further strengthened with the following
addition that addresses a latent tight loop bug on zero events being
returned:

index 7c3a42a..7760fb8 100644
--- a/engines/libaio.c
+++ b/engines/libaio.c
@@ -165,9 +165,9 @@ static int fio_libaio_getevents(struct thread_data
*td, unsigned int min,
r = io_getevents(ld->aio_ctx, actual_min,
max, ld->aio_events + events, lt);
}
- if (r >= 0)
+ if (r > 0)
events += r;
- else if (r == -EAGAIN) {
+ else if (r == 0 || r == -EAGAIN) {
fio_libaio_commit(td);
usleep(100);
} else if (r != -EINTR)
Regards,
Andrey

On Wed, Oct 8, 2014 at 4:00 PM, Jens Axboe <axboe@kernel.dk> wrote:
> The following changes since commit 15e3ca5b72b03f485abdda06bee9a9458dbd26a5:
>
> t/dedupe: avoid div-by-zero for all identical chunks (2014-10-06 21:07:25 -0600)
>
> are available in the git repository at:
>
> git://git.kernel.dk/fio.git master
>
> for you to fetch changes up to c05da82c1a427ce535af0c1b3e7a795330cf5061:
>
> gfio: remove warning on g_type_init() being deprecated (2014-10-07 20:46:26 -0600)
>
> ----------------------------------------------------------------
> Jens Axboe (9):
> t/dedupe: use generic blockdev_size() to get size
> t/dedupe: remove leftover linux/fs.h include
> t/dedupe: use fio abstracted OS_O_DIRECT
> Ignore lexer.h auto-generated file
> exp: fix issues around int vs size_t
> exp: small code cleanup
> libaio: commit on EAGAIN
> Makefile: fix gfio link for CONFIG_ARITHMETIC
> gfio: remove warning on g_type_init() being deprecated
>
> Slava Pestov (1):
> libaio: retry on -EINTR
>
> .gitignore | 1 +
> Makefile | 11 +++++-----
> engines/libaio.c | 7 +++++--
> exp/expression-parser.l | 14 +++++++++----
> gfio.c | 2 ++
> t/dedupe.c | 53 +++++++++++++++++++++++++++--------------------
> 6 files changed, 53 insertions(+), 35 deletions(-)
>
> ---
>
> Diff of recent changes:
>
> diff --git a/.gitignore b/.gitignore
> index c9d90fb..f13ec27 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -9,3 +9,4 @@
> /fio
> y.tab.*
> lex.yy.c
> +lexer.h
> diff --git a/Makefile b/Makefile
> index 5662015..5b03ec4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -159,13 +159,14 @@ OBJS = $(SOURCE:.c=.o)
>
> FIO_OBJS = $(OBJS) fio.o
>
> +GFIO_OBJS = $(OBJS) gfio.o graph.o tickmarks.o ghelpers.o goptions.o gerror.o \
> + gclient.o gcompat.o cairo_text_helpers.o printing.o
> +
> ifdef CONFIG_ARITHMETIC
> FIO_OBJS += lex.yy.o y.tab.o
> +GFIO_OBJS += lex.yy.o y.tab.o
> endif
>
> -GFIO_OBJS = $(OBJS) gfio.o graph.o tickmarks.o ghelpers.o goptions.o gerror.o \
> - gclient.o gcompat.o cairo_text_helpers.o printing.o
> -
> -include $(OBJS:.o=.d)
>
> T_SMALLOC_OBJS = t/stest.o
> @@ -194,13 +195,11 @@ T_BTRACE_FIO_OBJS += fifo.o lib/flist_sort.o t/log.o lib/linux-dev-lookup.o
> T_BTRACE_FIO_PROGS = t/btrace2fio
> endif
>
> -ifeq ($(CONFIG_TARGET_OS), Linux)
> T_DEDUPE_OBJS = t/dedupe.o
> T_DEDUPE_OBJS += lib/rbtree.o t/log.o mutex.o smalloc.o gettime.o crc/md5.o \
> memalign.o lib/bloom.o t/debug.o crc/xxhash.o crc/murmur3.o \
> crc/crc32c.o crc/crc32c-intel.o crc/fnv.o
> T_DEDUPE_PROGS = t/dedupe
> -endif
>
> T_OBJS = $(T_SMALLOC_OBJS)
> T_OBJS += $(T_IEEE_OBJS)
> @@ -351,7 +350,7 @@ t/dedupe: $(T_DEDUPE_OBJS)
> $(QUIET_LINK)$(CC) $(LDFLAGS) $(CFLAGS) -o $@ $(T_DEDUPE_OBJS) $(LIBS)
>
> clean: FORCE
> - @rm -f .depend $(FIO_OBJS) $(GFIO_OBJS) $(OBJS) $(T_OBJS) $(PROGS) $(T_PROGS) core.* core gfio FIO-VERSION-FILE *.d lib/*.d crc/*.d engines/*.d profiles/*.d t/*.d config-host.mak config-host.h y.tab.[ch] lex.yy.c exp/*.[do]
> + @rm -f .depend $(FIO_OBJS) $(GFIO_OBJS) $(OBJS) $(T_OBJS) $(PROGS) $(T_PROGS) core.* core gfio FIO-VERSION-FILE *.d lib/*.d crc/*.d engines/*.d profiles/*.d t/*.d config-host.mak config-host.h y.tab.[ch] lex.yy.c exp/*.[do] lexer.h
>
> distclean: clean FORCE
> @rm -f cscope.out fio.pdf fio_generate_plots.pdf fio2gnuplot.pdf
> diff --git a/engines/libaio.c b/engines/libaio.c
> index 6c5d73e..7c3a42a 100644
> --- a/engines/libaio.c
> +++ b/engines/libaio.c
> @@ -13,6 +13,8 @@
>
> #include "../fio.h"
>
> +static int fio_libaio_commit(struct thread_data *td);
> +
> struct libaio_data {
> io_context_t aio_ctx;
> struct io_event *aio_events;
> @@ -165,9 +167,10 @@ static int fio_libaio_getevents(struct thread_data *td, unsigned int min,
> }
> if (r >= 0)
> events += r;
> - else if (r == -EAGAIN)
> + else if (r == -EAGAIN) {
> + fio_libaio_commit(td);
> usleep(100);
> - else
> + } else if (r != -EINTR)
> break;
> } while (events < min);
>
> diff --git a/exp/expression-parser.l b/exp/expression-parser.l
> index 7d5e787..16e3873 100644
> --- a/exp/expression-parser.l
> +++ b/exp/expression-parser.l
> @@ -24,16 +24,22 @@
>
> #define YYSTYPE PARSER_VALUE_TYPE
>
> -extern int lexer_input(char* buffer, int *nbytes, int buffersize);
> +extern int lexer_input(char *buffer, size_t *nbytes, int buffersize);
>
> #undef YY_INPUT
> -#define YY_INPUT(buffer, bytes_read, bytes_requested) \
> - lexer_input((buffer), &(bytes_read), (bytes_requested))
> +#define YY_INPUT(buffer, bytes_read, bytes_requested) \
> +({ \
> + int __ret; \
> + size_t __bread = bytes_read; \
> + __ret = lexer_input((buffer), &__bread, (bytes_requested)); \
> + bytes_read = __bread; \
> + __ret; \
> +})
>
> extern int yyerror(long long *result, double *dresult,
> int *has_error, int *units_specified, const char *msg);
>
> -static void __attribute__((unused)) yyunput(int c,char *buf_ptr);
> +static void __attribute__((unused)) yyunput(int c, char *buf_ptr);
> static int __attribute__((unused)) input(void);
>
> #define set_suffix_value(yylval, i_val, d_val, has_d_val) \
> diff --git a/gfio.c b/gfio.c
> index 37c1db6..0e22cba 100644
> --- a/gfio.c
> +++ b/gfio.c
> @@ -1687,7 +1687,9 @@ static void init_ui(int *argc, char **argv[], struct gui *ui)
> gtk_init(argc, argv);
> settings = gtk_settings_get_default();
> gtk_settings_set_long_property(settings, "gtk_tooltip_timeout", 10, "gfio setting");
> +#if !GLIB_CHECK_VERSION(2, 36, 0)
> g_type_init();
> +#endif
> gdk_color_parse("#fffff4", &gfio_color_lightyellow);
> gdk_color_parse("white", &gfio_color_white);
>
> diff --git a/t/dedupe.c b/t/dedupe.c
> index 1577a69..030b4fc 100644
> --- a/t/dedupe.c
> +++ b/t/dedupe.c
> @@ -11,7 +11,6 @@
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> -#include <linux/fs.h>
> #include <fcntl.h>
> #include <string.h>
>
> @@ -78,17 +77,20 @@ static uint64_t total_size;
> static uint64_t cur_offset;
> static struct fio_mutex *size_lock;
>
> -static int dev_fd;
> +static struct fio_file file;
>
> -static uint64_t get_size(int fd, struct stat *sb)
> +static uint64_t get_size(struct fio_file *f, struct stat *sb)
> {
> uint64_t ret;
>
> if (S_ISBLK(sb->st_mode)) {
> - if (ioctl(fd, BLKGETSIZE64, &ret) < 0) {
> - perror("ioctl");
> + unsigned long long bytes;
> +
> + if (blockdev_size(f, &bytes)) {
> + log_err("dedupe: failed getting bdev size\n");
> return 0;
> }
> + ret = bytes;
> } else
> ret = sb->st_size;
>
> @@ -164,10 +166,10 @@ static int col_check(struct chunk *c, struct item *i)
> ibuf = fio_memalign(blocksize, blocksize);
>
> e = flist_entry(c->extent_list[0].next, struct extent, list);
> - if (read_block(dev_fd, cbuf, e->offset))
> + if (read_block(file.fd, cbuf, e->offset))
> goto out;
>
> - if (read_block(dev_fd, ibuf, i->offset))
> + if (read_block(file.fd, ibuf, i->offset))
> goto out;
>
> ret = memcmp(ibuf, cbuf, blocksize);
> @@ -372,8 +374,8 @@ static void show_progress(struct worker_thread *threads, unsigned long total)
> };
> }
>
> -static int run_dedupe_threads(int fd, uint64_t dev_size, uint64_t *nextents,
> - uint64_t *nchunks)
> +static int run_dedupe_threads(struct fio_file *f, uint64_t dev_size,
> + uint64_t *nextents, uint64_t *nchunks)
> {
> struct worker_thread *threads;
> unsigned long nitems, total_items;
> @@ -386,7 +388,7 @@ static int run_dedupe_threads(int fd, uint64_t dev_size, uint64_t *nextents,
>
> threads = malloc(num_threads * sizeof(struct worker_thread));
> for (i = 0; i < num_threads; i++) {
> - threads[i].fd = fd;
> + threads[i].fd = f->fd;
> threads[i].items = 0;
> threads[i].err = 0;
> threads[i].done = 0;
> @@ -429,25 +431,25 @@ static int dedupe_check(const char *filename, uint64_t *nextents,
>
> flags = O_RDONLY;
> if (odirect)
> - flags |= O_DIRECT;
> + flags |= OS_O_DIRECT;
> +
> + memset(&file, 0, sizeof(file));
> + file.file_name = strdup(filename);
>
> - dev_fd = open(filename, flags);
> - if (dev_fd == -1) {
> + file.fd = open(filename, flags);
> + if (file.fd == -1) {
> perror("open");
> - return 1;
> + goto err;
> }
>
> - if (fstat(dev_fd, &sb) < 0) {
> + if (fstat(file.fd, &sb) < 0) {
> perror("fstat");
> - close(dev_fd);
> - return 1;
> + goto err;
> }
>
> - dev_size = get_size(dev_fd, &sb);
> - if (!dev_size) {
> - close(dev_fd);
> - return 1;
> - }
> + dev_size = get_size(&file, &sb);
> + if (!dev_size)
> + goto err;
>
> if (use_bloom) {
> uint64_t bloom_entries;
> @@ -458,7 +460,12 @@ static int dedupe_check(const char *filename, uint64_t *nextents,
>
> printf("Will check <%s>, size <%llu>, using %u threads\n", filename, (unsigned long long) dev_size, num_threads);
>
> - return run_dedupe_threads(dev_fd, dev_size, nextents, nchunks);
> + return run_dedupe_threads(&file, dev_size, nextents, nchunks);
> +err:
> + if (file.fd != -1)
> + close(file.fd);
> + free(file.file_name);
> + return 1;
> }
>
> static void show_chunk(struct chunk *c)
> --
> To unsubscribe from this list: send the line "unsubscribe fio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html