On 13/10/10 08:31, Alexis Berlemont wrote: >> 1. Buffer management is badly broken. It is not possible to run any >> acquisition command that wraps around in the ring buffer. That can be >> simply reproduced with: >> >> cmd_read -v d analogy0 -s 0 -S 0 >> >> after a while it will fail with: "cmd_read: a4l_read failed (ret=-32)". >> In dmesg the driver reports: "Analogy: MITE: DME overwrite of free area". > > OK. Maybe, it is not Analogy's buffer management; it may be specific > to the NI driver. Did you try to reproduce the bug with another driver > (analogy_fake for example)? It may be due to some fix I made this > summer (which is surprising, I thought I tested the change quite > exhaustively... Sorry for that).
The problem is probably in the NI driver. I'm unable to reproduce the bug with the analogy_fake driver. See attached test script. >> 2. Buffer is kept memory mapped after the mapping process dies. If a >> process mapping a device buffer dies before un-mapping the buffer, it is >> not possible to reconfigure the bnuffer. a4l_set_bufsize fails with >> error code 32 and dmesg read "Analogy: a4l_ioctl_bufcfg: please unmap >> before configuring buffer". Sinche the mapping process died I do not >> know how to do this. >> > With a 2.4.x release, did you manage to reproduce the bug? I'm unable to reproduce the bug with xenomai 2.5.3. See attached test. It depends on a patch to make the --read-buffer-size and --write-buffer-size options to analogy_config to effectively work. Also attached. I do not have a 2.5.4 version handy. Cheers, -- Daniele
test_buffer_bug.sh
Description: Bourne shell script
test_mmap_bug.sh
Description: Bourne shell script
/** * analogy for linux - input command test program */ #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <sys/mman.h> #include <errno.h> #include <getopt.h> #include <string.h> #include <analogy/analogy.h> /* default device name */ #define DEVICE "analogy0" #define DEBUG(frmt, args...) fprintf(stderr, frmt"\n", ##args) #define ERROR(frmt, args...) fprintf(stderr, frmt"\n", ##args) int main(int argc, char **argv) { int rv = 0; unsigned long bufsize; void *map = NULL; a4l_desc_t dsc; char *filename = argv[1]; /* open the device */ rv = a4l_open(&dsc, filename); if (rv < 0) { ERROR("analogy open"); return rv; } /* cancel any command which might be in progress */ a4l_snd_cancel(&dsc, dsc.idx_read_subd); /* get buffer size to map */ rv = a4l_get_bufsize(&dsc, dsc.idx_read_subd, &bufsize); if (rv < 0) { ERROR("analogy get bufsize"); return rv; } /* map read subdevice buffer */ rv = a4l_mmap(&dsc, dsc.idx_read_subd, bufsize, &map); if (rv < 0) { ERROR("analogy mmap"); return rv; } if (argc > 2) { DEBUG("BUG!"); } else { /* unmap subdevice buffer */ munmap(map, bufsize); } /* close file descriptor */ a4l_close(&dsc); exit(0); }
commit e7ac38f3f45e439a8d383f3b4adafbb97bf543fe Author: Daniele Nicolodi <nicol...@science.unitn.it> Date: Wed Oct 13 16:03:30 2010 +0200 Make the --read-buffer-size and --write-buffer-size otpions effective. diff --git a/src/utils/analogy/analogy_config.c b/src/utils/analogy/analogy_config.c index 1482322..2ce3ddd 100644 --- a/src/utils/analogy/analogy_config.c +++ b/src/utils/analogy/analogy_config.c @@ -31,28 +31,25 @@ #include <analogy/analogy.h> -/* Declare precompilation constants */ -#define __NBMIN_ARG 2 -#define __NBMAX_ARG 3 -#define __OPTS_DELIMITER "," +enum action { + ATTACH_DRIVER, + DETACH_DRIVER, + CONFIGURE_BUFFER_SIZE, +}; -/* Declare prog variables */ -int vlevel = 1; -int unatt_act = 0; -struct option a4l_conf_opts[] = { - {"help", no_argument, NULL, 'h'}, - {"verbose", no_argument, NULL, 'v'}, - {"quiet", no_argument, NULL, 'q'}, - {"version", no_argument, NULL, 'V'}, - {"remove", no_argument, NULL, 'r'}, - {"read-buffer-size", required_argument, NULL, 'R'}, - {"write-buffer-size", required_argument, NULL, 'W'}, - {0}, +enum subdevice { + READ_SUBDEVICE, + WRITE_SUBDEVICE, }; +/* Verbosity level */ +int vlevel = 1; + +/* Driver specific Options delimiter */ +#define DELIMITER "," + int compute_opts(char *opts, unsigned int *nb, unsigned long *res) { - int ret = 0, len, ofs; /* Check arg and inits it */ @@ -67,7 +64,7 @@ int compute_opts(char *opts, unsigned int *nb, unsigned long *res) do { (*nb)++; len = strlen(opts); - ofs = strcspn(opts, __OPTS_DELIMITER); + ofs = strcspn(opts, DELIMITER); if (res != NULL) { res[(*nb) - 1] = strtoul(opts, NULL, 0); if (errno != 0) { @@ -104,79 +101,57 @@ void do_print_usage(void) "\t\t -W, --write-buffer-size: write buffer size in kB\n"); } -int main(int argc, char *argv[]) +int detach_driver(const char *devfile) { - int c; - char *devfile; - a4l_lnkdesc_t lnkdsc; - int chk_nb, ret = 0, fd = -1; - - /* Init the descriptor structure */ - memset(&lnkdsc, 0, sizeof(a4l_lnkdesc_t)); - - /* Compute arguments */ - while ((c = - getopt_long(argc, argv, "hvqVrR:W:", a4l_conf_opts, - NULL)) >= 0) { - switch (c) { - case 'h': - do_print_usage(); - goto out_a4l_config; - case 'v': - vlevel = 2; - break; - case 'q': - vlevel = 0; - break; - case 'V': - do_print_version(); - goto out_a4l_config; - case 'r': - unatt_act = 1; - break; - case 'R': - break; - case 'W': - break; - } + /* Open device */ + int fd = a4l_sys_open(devfile); + if (fd < 0) { + fprintf(stderr, "a4l_open failed ret=%d\n", fd); + return fd; } - /* Check the last arguments */ - chk_nb = (unatt_act == 0) ? __NBMIN_ARG : __NBMIN_ARG - 1; - if (argc - optind < chk_nb) { - do_print_usage(); - goto out_a4l_config; + /* Detach driver */ + int ret = a4l_sys_detach(fd); + if (ret < 0) { + fprintf(stderr, "a4l_snd_detach failed ret=%d\n", ret); + return ret; } + + /* Close device */ + a4l_sys_close(fd); - /* Get the device file name */ - devfile = argv[optind]; + return 0; +} + +int attach_driver(const char *devfile, char *driver, char *opts) +{ + int ret; + a4l_lnkdesc_t lnkdsc; + + /* Init the descriptor structure */ + memset(&lnkdsc, 0, sizeof(a4l_lnkdesc_t)); /* Fill the descriptor with the driver name */ - if (unatt_act == 0) { - lnkdsc.bname = argv[optind + 1]; - lnkdsc.bname_size = strlen(argv[optind + 1]); - } + lnkdsc.bname = driver; + lnkdsc.bname_size = strlen(driver); - /* Handle the last arguments: the driver-specific args */ - if (unatt_act == 1 || argc - optind != __NBMAX_ARG) - lnkdsc.opts_size = 0; - else { - char *opts = argv[optind + __NBMAX_ARG - 1]; + /* Handle driver specific options */ + lnkdsc.opts_size = 0; + if (opts != NULL) { if ((ret = compute_opts(opts, &lnkdsc.opts_size, NULL)) < 0) { fprintf(stderr, "analogy_config: specific-driver options recovery failed\n"); fprintf(stderr, "\twarning: specific-driver options must be integer value\n"); do_print_usage(); - goto out_a4l_config; + return -1; } lnkdsc.opts = malloc(lnkdsc.opts_size); if (lnkdsc.opts == NULL) { fprintf(stderr, "analogy_config: memory allocation failed\n"); - ret = -ENOMEM; - goto out_a4l_config; + return -ENOMEM; } if ((ret = @@ -186,38 +161,165 @@ int main(int argc, char *argv[]) fprintf(stderr, "\twarning: specific-driver options must be integer value\n"); do_print_usage(); - goto out_a4l_config; + return -1; } } /* Open the specified file */ - fd = a4l_sys_open(devfile); + int fd = a4l_sys_open(devfile); if (fd < 0) { - ret = fd; - fprintf(stderr, "analogy_config: a4l_open failed ret=%d\n", - ret); - goto out_a4l_config; + fprintf(stderr, "a4l_open failed ret=%d\n", fd); + return fd; } - /* Trigger the ioctl */ - if (unatt_act == 0) - ret = a4l_sys_attach(fd, &lnkdsc); - else - ret = a4l_sys_detach(fd); + /* Attach driver */ + ret = a4l_sys_attach(fd, &lnkdsc); if (ret < 0) { - fprintf(stderr, "analogy_config: %s failed ret=%d\n", - (unatt_act == - 0) ? "a4l_snd_attach" : "a4l_snd_detach", ret); - goto out_a4l_config; + fprintf(stderr, "a4l_snd_attach failed ret=%d\n", ret); + return ret; } -out_a4l_config: + /* Close device */ + a4l_sys_close(fd); - if (fd >= 0) - a4l_sys_close(fd); + free(lnkdsc.opts); - if (lnkdsc.opts != NULL) - free(lnkdsc.opts); + return 0; +} - return ret; +int configure_buffer_size(char *device, int subd, unsigned long size) +{ + /* Open device */ + a4l_desc_t dsc; + int ret = a4l_open(&dsc, device); + if (ret < 0) { + fprintf(stderr, "a4l_open error ret=%d\n", ret); + return ret; + } + + /* Choose subdevice */ + unsigned long subd_idx = (subd == READ_SUBDEVICE ? dsc.idx_read_subd : dsc.idx_write_subd); + const char *name = (subd == READ_SUBDEVICE ? "read" : "write"); + + /* Get buffer size */ + unsigned long bufsize; + ret = a4l_get_bufsize(&dsc, subd_idx, &bufsize); + if (ret < 0) { + fprintf(stderr, "a4l_get_bufsize error ret=%d\n", ret); + return ret; + } + + if (size > 0) { + + /* Configure buffer size */ + ret = a4l_set_bufsize(&dsc, subd_idx, size); + if (ret < 0) { + fprintf(stderr, "a4l_set_bufsize error ret=%d\n", ret); + return -1; + } + + /* Report buffer size */ + if (vlevel > 1) { + fprintf(stdout, "old %s buffer size = %lu\n", name, bufsize); + fprintf(stdout, "new %s buffer size = %lu\n", name, size); + } + + } else { + + /* Report buffer size */ + if (vlevel > 0) + fprintf(stdout, "%s buffer size = %lu\n", name, bufsize); + else + fprintf(stdout, "%lu\n", bufsize); + + } + + /* Close device */ + ret = a4l_close(&dsc); + if (ret < 0) { + fprintf(stderr, "a4l_close error ret=%d\n", ret); + return -1; + } + + return 0; +} + +int main(int argc, char *argv[]) +{ + int c; + int action; + int subdevice = -1; + unsigned long size = 0; + + /* Default action is to attach a driver */ + action = ATTACH_DRIVER; + + struct option options[] = { + {"help", no_argument, NULL, 'h'}, + {"verbose", no_argument, NULL, 'v'}, + {"quiet", no_argument, NULL, 'q'}, + {"version", no_argument, NULL, 'V'}, + {"remove", no_argument, NULL, 'r'}, + {"read-buffer-size", optional_argument, NULL, 'R'}, + {"write-buffer-size", optional_argument, NULL, 'W'}, + { 0 }, + }; + + /* Compute arguments */ + while ((c = + getopt_long(argc, argv, "hvqVrR::W::", options, NULL)) >= 0) { + switch (c) { + case 'h': + do_print_usage(); + return 0; + case 'v': + vlevel = 2; + break; + case 'q': + vlevel = 0; + break; + case 'V': + do_print_version(); + return 0; + case 'r': + action = DETACH_DRIVER; + break; + case 'R': + action = CONFIGURE_BUFFER_SIZE; + subdevice = READ_SUBDEVICE; + if (optarg) + size = strtoul(optarg, NULL, 10); + break; + case 'W': + action = CONFIGURE_BUFFER_SIZE; + subdevice = WRITE_SUBDEVICE; + if (optarg) + size = strtoul(optarg, NULL, 10); + break; + } + } + + switch (action) { + case ATTACH_DRIVER: + if ((argc - optind) == 2) + return attach_driver(argv[optind], argv[optind+1], NULL); + if ((argc - optind) == 3) + return attach_driver(argv[optind], argv[optind+1], argv[optind+2]); + do_print_usage(); + return -1; + + case DETACH_DRIVER: + if ((argc - optind) == 1) + return detach_driver(argv[optind]); + do_print_usage(); + return -1; + + case CONFIGURE_BUFFER_SIZE: + if ((argc - optind) == 1) + return configure_buffer_size(argv[optind], subdevice, size); + do_print_usage(); + return -1; + } + + return 0; }
_______________________________________________ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core