Re: [PATCH v2 liburing] Test consistent file position updates

[Dylan Yudaken <dylany@fb.com>]

On Mon, 2022-02-21 at 09:29 -0700, Jens Axboe wrote:
> On 2/21/22 7:18 AM, Dylan Yudaken wrote:
> 
> > +
> > +#define FILE_SIZE 10000
> > +#define QUEUE_SIZE 4096
> 
> Seems huge? 
> 

The problems occur a little bit randomly, but certainly the more in
flight at a time the worse it is. I'll reduce it to maybe 1/4? it's not
particularly slow.

> > +static int
> > +test_read(struct io_uring *ring, bool async, bool link, int
> > blocksize)
> 
> liburing and the kernel tend to use:
> 
> static int test_read(struct io_uring *ring, bool async, bool link,
>                      int blocksize)
> {
> }
> 
> if we go over 80 chars, with the reasoning being that a grep will
> show
> you the return type at least.

makes sense!

> 
> > +{
> > +       int ret, fd, i;
> > +       bool done = false;
> > +       struct io_uring_sqe *sqe;
> > +       struct io_uring_cqe *cqe;
> > +       loff_t current, expected = 0;
> > +       int count_ok;
> > +       int count_0 = 0, count_1 = 0;
> > +       unsigned char buff[QUEUE_SIZE * blocksize];
> > +       unsigned char reordered[QUEUE_SIZE * blocksize];
> > +
> > +       create_file(".test_read", FILE_SIZE);
> > +       fd = open(".test_read", O_RDONLY);
> > +       unlink(".test_read");
> > +       assert(fd >= 0);
> > +
> > +       while (!done) {
> > +               for (i = 0; i < QUEUE_SIZE; ++i) {
> > +                       sqe = io_uring_get_sqe(ring);
> > +                       if (!sqe) {
> > +                               fprintf(stderr, "no sqe\n");
> > +                               return -1;
> > +                       }
> > +                       io_uring_prep_read(sqe, fd,
> > +                                       buff + i * blocksize,
> > +                                       blocksize, -1);
> > +                       sqe->user_data = i;
> > +                       if (async)
> > +                               sqe->flags |= IOSQE_ASYNC;
> > +                       if (link && i != QUEUE_SIZE - 1)
> > +                               sqe->flags |= IOSQE_IO_LINK;
> > +               }
> > +               ret = io_uring_submit_and_wait(ring, QUEUE_SIZE);
> > +               if (ret != QUEUE_SIZE) {
> > +                       fprintf(stderr, "submit failed: %d\n",
> > ret);
> > +                       return 1;
> > +               }
> > +               count_ok  = 0;
> > +               for (i = 0; i < QUEUE_SIZE; ++i) {
> > +                       int res;
> > +
> > +                       ret = io_uring_peek_cqe(ring, &cqe);
> > +                       if (ret) {
> > +                               fprintf(stderr, "peek failed:
> > %d\n", ret);
> > +                               return ret;
> > +                       }
> > +                       assert(cqe->user_data < QUEUE_SIZE);
> > +                       memcpy(reordered + count_ok
> > +                               , buff + cqe->user_data * blocksize
> > +                               , blocksize);
> 
>                         memcpy(reordered + count_ok,
>                                 buff + cqe->user_data * blocksize,
> blocksize);
> 
> > +static int
> > +test_write(struct io_uring *ring, bool async, bool link, int
> > blocksize)
> > +{
> > +       int ret, fd, i;
> > +       struct io_uring_sqe *sqe;
> > +       struct io_uring_cqe *cqe;
> > +       bool fail = false;
> > +       loff_t current;
> > +       char data[blocksize+1];
> > +       char readbuff[QUEUE_SIZE*blocksize+1];
> > +
> > +       fd = open(".test_write", O_RDWR | O_CREAT, 0644);
> > +       unlink(".test_write");
> > +       assert(fd >= 0);
> > +
> > +       for(i = 0; i < blocksize; i++) {
> > +               data[i] = 'A' + i;
> > +       }
> > +       data[blocksize] = '\0';
> > +
> > +       for (i = 0; i < QUEUE_SIZE; ++i) {
> > +               sqe = io_uring_get_sqe(ring);
> > +               if (!sqe) {
> > +                       fprintf(stderr, "no sqe\n");
> > +                       return -1;
> > +               }
> > +               io_uring_prep_write(sqe, fd, data + (i %
> > blocksize), 1, -1);
> > +               sqe->user_data = 1;
> > +               if (async)
> > +                       sqe->flags |= IOSQE_ASYNC;
> > +               if (link && i != QUEUE_SIZE - 1)
> > +                       sqe->flags |= IOSQE_IO_LINK;
> > +       }
> > +       ret = io_uring_submit_and_wait(ring, QUEUE_SIZE);
> > +       if (ret != QUEUE_SIZE) {
> > +               fprintf(stderr, "submit failed: %d\n", ret);
> > +               return 1;
> > +       }
> > +       for (i = 0; i < QUEUE_SIZE; ++i) {
> > +               int res;
> > +
> > +               ret = io_uring_peek_cqe(ring, &cqe);
> > +               res = cqe->res;
> > +               if (ret) {
> > +                       fprintf(stderr, "peek failed: %d\n", ret);
> > +                       return ret;
> > +               }
> > +               io_uring_cqe_seen(ring, cqe);
> > +               if (!fail && res != 1) {
> > +                       fprintf(stderr, "bad result %d\n", res);
> > +                       fail = true;
> > +               }
> > +       }
> > +       current = lseek(fd, 0, SEEK_CUR);
> > +       if (current != QUEUE_SIZE) {
> > +               fprintf(stderr,
> > +                       "f_pos incorrect, expected %ld have %d\n",
> > +                       current,
> > +                       QUEUE_SIZE);
> > +               fail = true;
> 
>                 fprintf(stderr, "f_pos incorrect, expected %ld have
> %d\n",
>                                 current, QUEUE_SIZE);
> 
> > +int main(int argc, char *argv[])
> > +{
> > +       struct io_uring ring;
> > +       int ret;
> > +       int failed = 0;
> > +       int blocksizes[] = {1, 8, 15, 0};
> > +
> > +       if (argc > 1)
> > +               return 0;
> > +
> > +       ret = io_uring_queue_init(QUEUE_SIZE, &ring, 0);
> > +       if (ret) {
> > +               fprintf(stderr, "ring setup failed\n");
> > +               return 1;
> > +       }
> > +
> > +       for (int *blocksize = blocksizes; *blocksize; blocksize++)
> > {
> > +       for (int async = 0; async < 2; async++) {
> > +       for (int link = 0; link < 2; link++) {
> > +       for (int write = 0; write < 2; write++) {
> > +               fprintf(stderr, "*********\n");
> > +               ret = write
> > +                       ? test_write(&ring, !!async, !!link,
> > *blocksize)
> > +                       : test_read(&ring, !!async, !!link,
> > *blocksize);
> > +               fprintf(stderr, "test %s async=%d link=%d
> > blocksize=%d:\t%s\n",
> > +                       write ? "write":"read",
> > +                       async, link,
> > +                       *blocksize,
> > +                       ret ? "failed" : "passed");
> 
> The normal procedure for tests and printing output is:
> 
> - Be silent on success, otherwise it's just noise to look over when
>   doing runs. There are some ancient tests that don't honor this, but
>   generally the rest of them do.

Ok!

> 
> - If one test fails, stop further tests.
> 
> > +       }
> > +       }
> > +       }
> 
> Indentation is wonky here.
> 

Yes - on this one I wasn't sure the best approach, as it's 4 nested for
loops in order to generate a large set of test cases. I did not feel
like indenting them was valuable. That being said it's probably worth
it just for the consistency - so I will do that.