[RHEL7,COMMIT] fuse kio: Extract fiemap iteration from fiemap_worker() to separate function

Submitted by Konstantin Khorenko on Sept. 5, 2018, 10:06 a.m.

Details

Message ID 201809051006.w85A6Z8w010692@finist_ce7.work
State New
Series "fuse kio: Extract fiemap iteration from fiemap_worker() to separate function"
Headers show

Commit Message

Konstantin Khorenko Sept. 5, 2018, 10:06 a.m.
The commit is pushed to "branch-rh7-3.10.0-862.11.6.vz7.71.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.11.6.vz7.71.8
------>
commit 132093a91d2cdce14d81f741b09820ac67613845
Author: Kirill Tkhai <ktkhai@virtuozzo.com>
Date:   Wed Sep 5 13:06:34 2018 +0300

    fuse kio: Extract fiemap iteration from fiemap_worker() to separate function
    
    This is just refactoring, which just moves the cycle
    to new function. It will be used in next patches.
    
    Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
    Acked-by: Alexey Kuznetsov <kuznet@virtuozzo.com>
    
    =====================
    Patchset description:
    
    Optimize fiemap ioctl
    
    https://jira.sw.ru/browse/HCI-90
    
    Summary:
      This patch set optimizes fiemap ioctl by removing
      kthread creation. Instead of this, static work
      is used, so we safe some time on copy_process().
    
    Fiemap does not require separate kthread, since
    the most time the kthread is spending in waiting
    for fiter->ireq.iocount becomes 0. Instead of this,
    the generic kthread could queue another fiemap
    requests at this time. This is the thing the patch set
    introduces.
    
    Note, that we had a kthread for every fiemap request,
    and this may look more scalable. But this is not true,
    since the actions, fiemap does, is pretty small. So,
    I think for the most workload the single fiemap work
    is enough. If we meet a workload, where the single
    work is not enough, it will be pretty easy to make
    fiemap_work just as an array in struct pcs_cluster_core
    (to make it per-node or even per-cpu). But I think,
    it's not necessary at least till main_job or completion_job
    are per-node or per-cpu (fiemap requests are small subset
    of all requests going through main_job).
    
    https://github.com/shekkbuilder/fiemap/blob/master/fiemap.c
    code was taken as a base for the performance test and modified.
    The below is results and the test's code.
    
    Time of test execution on 3 extents-file (just randomly chosen
    number of extents):
    
    Before: real 0m11.069s
    After:  real 0m9.112s
    
    It became 17% faster, it was 21% slower.
    
    Kirill Tkhai (7):
          fuse kio: Extract fiemap iteration from fiemap_worker() to separate function
          fuse kio: Move it variable from stack to struct fiemap_iterator
          fuse kio: Kill fiemap_worker() thread
          fuse kio: Move position advance in fiemap_process_one()
          fuse kio: Move fiter ireq iocount assignment
          fuse kio: Introduce fiemap_work
          fuse kio: Async queueing of fiemap from work
    
    8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<8<
    
    #include <stdlib.h>
    #include <stdio.h>
    #include <string.h>
    #include <unistd.h>
    #include <fcntl.h>
    #include <sys/types.h>
    #include <sys/stat.h>
    #include <sys/ioctl.h>
    #include <linux/fs.h>
    #include <linux/fiemap.h>
    
    #define BUF_SIZE (4 * 4096)
    void syntax(char **argv)
    {
            fprintf(stderr, "%s [filename]...\n",argv[0]);
    }
    struct fiemap *read_fiemap(int fd, struct fiemap *fiemap)
    {
            int extents_size;
            int i;
    
            memset(fiemap, 0, sizeof(struct fiemap));
            fiemap->fm_start = 0;
            fiemap->fm_length = ~0;         /* Lazy */
            fiemap->fm_flags = FIEMAP_FLAG_SYNC;
            fiemap->fm_extent_count = 0;
            fiemap->fm_mapped_extents = 0;
    
            /* Find out how many extents there are */
            if (ioctl(fd, FS_IOC_FIEMAP, fiemap) < 0) {
                    fprintf(stderr, "fiemap ioctl() failed\n");
                    return NULL;
            }
    
            /* Read in the extents */
            extents_size = sizeof(struct fiemap_extent) *
                                  (fiemap->fm_mapped_extents);
            if (extents_size > BUF_SIZE) {
                    fprintf(stderr, "Too small buffer\n");
                    return NULL;
            }
    
            memset(fiemap->fm_extents, 0, extents_size);
            fiemap->fm_extent_count = fiemap->fm_mapped_extents;
            fiemap->fm_mapped_extents = 0;
    
            if (ioctl(fd, FS_IOC_FIEMAP, fiemap) < 0) {
                    fprintf(stderr, "fiemap ioctl() failed\n");
                    return NULL;
            }
            return fiemap;
    }
    void dump_fiemap(struct fiemap *fiemap, char *filename)
    {
            int i;
            printf("File %s has %d extents:\n",filename, fiemap->fm_mapped_extents);
            printf("#\tLogical          Physical         Length           Flags\n");
            for (i=0;i<fiemap->fm_mapped_extents;i++) {
                    printf("%d:\t%-16.16llx %-16.16llx %-16.16llx %-4.4x\n",
                            i,
                            fiemap->fm_extents[i].fe_logical,
                            fiemap->fm_extents[i].fe_physical,
                            fiemap->fm_extents[i].fe_length,
                            fiemap->fm_extents[i].fe_flags);
            }
            printf("\n");
    }
    int main(int argc, char **argv)
    {
            int i;
            if (argc < 2) {
                    syntax(argv);
                    exit(EXIT_FAILURE);
            }
            for (i=1;i<argc;i++) {
                    int fd, j;
                    if ((fd = open(argv[i], O_RDONLY)) < 0) {
                            fprintf(stderr, "Cannot open file %s\n", argv[i]);
                    }
                    else {
                            struct fiemap *fiemap;
                            if ((fiemap = (struct fiemap*)malloc(sizeof(struct fiemap) + BUF_SIZE)) == NULL) {
                                    fprintf(stderr, "Out of memory allocating fiemap\n");
                                    exit(EXIT_FAILURE);
                            }
                            for (j = 0; j < 50000; j++) {
                                    if ((fiemap = read_fiemap(fd, fiemap)) == NULL) {
                                            fprintf(stderr, "read err\n");
                                            exit(EXIT_FAILURE);
                                    }
                            }
                            dump_fiemap(fiemap, argv[i]);
                            close(fd);
                    }
            }
            exit(EXIT_SUCCESS);
    }
---
 fs/fuse/kio/pcs/pcs_cluster.c | 78 ++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 35 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/kio/pcs/pcs_cluster.c b/fs/fuse/kio/pcs/pcs_cluster.c
index e243b11d0695..088c18fccb18 100644
--- a/fs/fuse/kio/pcs/pcs_cluster.c
+++ b/fs/fuse/kio/pcs/pcs_cluster.c
@@ -164,43 +164,12 @@  static void xfer_fiemap_extents(struct fiemap_iterator * iter, u64 pos, char * b
 	}
 }
 
-static int fiemap_worker(void * arg)
+static void fiemap_process_one(struct fiemap_iterator *fiter)
 {
-	struct pcs_int_request * orig_ireq = arg;
-	struct pcs_dentry_info * di;
-	struct fiemap_iterator * fiter;
-	struct iov_iter it;
+	struct pcs_int_request *orig_ireq = fiter->orig_ireq;
+	struct pcs_dentry_info *di = orig_ireq->dentry;
 	u64 pos, end;
 
-	fiter = kmalloc(sizeof(struct fiemap_iterator), GFP_KERNEL);
-	if (fiter == NULL) {
-		pcs_set_local_error(&orig_ireq->error, PCS_ERR_NOMEM);
-		ireq_complete(orig_ireq);
-		return 0;
-	}
-
-	fiter->orig_ireq = orig_ireq;
-	init_waitqueue_head(&fiter->wq);
-	di = orig_ireq->dentry;
-	ireq_init(di, &fiter->ireq);
-	fiter->ireq.type = PCS_IREQ_API;
-	fiter->ireq.apireq.req = &fiter->apireq;
-	fiter->ireq.completion_data.parent = NULL;
-	fiter->ireq.complete_cb = fiemap_iter_done;
-	fiter->apireq.datasource = fiter;
-	fiter->apireq.get_iter = fiemap_get_iter;
-	fiter->apireq.complete = NULL;
-	fiter->buffer = kvmalloc(PCS_FIEMAP_BUFSIZE, GFP_KERNEL);
-	if (fiter->buffer == NULL) {
-		pcs_set_local_error(&orig_ireq->error, PCS_ERR_NOMEM);
-		ireq_complete(orig_ireq);
-		kfree(fiter);
-		return 0;
-	}
-	fiter->fiemap_max = orig_ireq->apireq.aux;
-	orig_ireq->apireq.req->get_iter(orig_ireq->apireq.req->datasource, 0, &it);
-	fiter->mapped = &((struct fiemap*)it.data)->fm_mapped_extents;
-
 	pos = fiter->orig_ireq->apireq.req->pos;
 	end = pos + fiter->orig_ireq->apireq.req->size;
 	while (pos < end) {
@@ -256,11 +225,50 @@  static int fiemap_worker(void * arg)
 
 		pos += fiter->apireq.size;
 	}
-
 out:
 	kvfree(fiter->buffer);
 	kfree(fiter);
 	ireq_complete(orig_ireq);
+}
+
+static int fiemap_worker(void * arg)
+{
+	struct pcs_int_request * orig_ireq = arg;
+	struct pcs_dentry_info * di;
+	struct fiemap_iterator * fiter;
+	struct iov_iter it;
+
+	fiter = kmalloc(sizeof(struct fiemap_iterator), GFP_KERNEL);
+	if (fiter == NULL) {
+		pcs_set_local_error(&orig_ireq->error, PCS_ERR_NOMEM);
+		ireq_complete(orig_ireq);
+		return 0;
+	}
+
+	fiter->orig_ireq = orig_ireq;
+	init_waitqueue_head(&fiter->wq);
+	di = orig_ireq->dentry;
+	ireq_init(di, &fiter->ireq);
+	fiter->ireq.type = PCS_IREQ_API;
+	fiter->ireq.apireq.req = &fiter->apireq;
+	fiter->ireq.completion_data.parent = NULL;
+	fiter->ireq.complete_cb = fiemap_iter_done;
+	fiter->apireq.datasource = fiter;
+	fiter->apireq.get_iter = fiemap_get_iter;
+	fiter->apireq.complete = NULL;
+	fiter->buffer = kvmalloc(PCS_FIEMAP_BUFSIZE, GFP_KERNEL);
+	if (fiter->buffer == NULL) {
+		pcs_set_local_error(&orig_ireq->error, PCS_ERR_NOMEM);
+		ireq_complete(orig_ireq);
+		kfree(fiter);
+		return 0;
+	}
+	fiter->fiemap_max = orig_ireq->apireq.aux;
+	orig_ireq->apireq.req->get_iter(orig_ireq->apireq.req->datasource, 0, &it);
+	fiter->mapped = &((struct fiemap*)it.data)->fm_mapped_extents;
+
+	fiemap_process_one(fiter);
+
 	return 0;
 }