From 65b98d18b73dfc41c14b3c5ecac61968f1b94aea Mon Sep 17 00:00:00 2001 From: Alessandro Rubini <rubini@gnudd.com> Date: Fri, 9 Nov 2012 17:09:07 +0100 Subject: [PATCH] core: incompatible: buffer->alloc_block must allocate the control This is an incompatible change for buffer authors. We don't think there are any out-of-tree buffers, though. The allocation function must allocate the control by itself, instead of receiving it from outside. The reason why the control was passed doesn't hold any more, and the upcoming PF_ZIO buffer will greatly benefit from this. Also, now alloc_block returns NULL on error, so it can be used directly by the caller (it used to be ERR_PTR). Signed-off-by: Alessandro Rubini <rubini@gnudd.com> Acked-by: Federico Vaga <federico.vaga@gmail.com> --- Documentation/zio/buffer.txt | 7 ++++--- buffers/zio-buf-kmalloc.c | 10 +++++----- buffers/zio-buf-vmalloc.c | 20 +++++++++----------- doc/zio-manual.in | 9 ++++++--- include/linux/zio-buffer.h | 7 +++++-- zio-cdev.c | 20 ++++++-------------- zio-sys.c | 30 +++++++++--------------------- 7 files changed, 44 insertions(+), 59 deletions(-) diff --git a/Documentation/zio/buffer.txt b/Documentation/zio/buffer.txt index aee30d8..de3eb3e 100644 --- a/Documentation/zio/buffer.txt +++ b/Documentation/zio/buffer.txt @@ -53,7 +53,8 @@ only zio_bi, so complex buffer structures must contain the zio_bi structure and use container_of() to access the private enclosing structure. create() can use default attributes of its buffer type at creation time; a copy for this specific instance is created by -zio-core after the function returns. +zio-core after the function returns. A failed creation must return +ERR_PTR(-ENOMEM) or similar. destroy() deallocates a buffer instance. ZIO calls destroy() when the channel is unregistered from ZIO or when the user assigns a different @@ -66,7 +67,6 @@ handled by ZIO, so it couldn't call destroy on close even if it wanted to. struct zio_block *(*alloc_block)(struct zio_bi *bi, - struct zio_control *ctrl, size_t datalen, gfp_t gfp); void (*free_block)(struct zio_bi *bi, struct zio_block *block); @@ -79,7 +79,8 @@ structure hosting a block is zio_block which contain both data (samples) and control information. If needed, buffers may use a more complex structure, which will include zio_block, which is the only structure that ZIO handles; by using container_of you can retrieve the -enclosing complex structure used in your buffer +enclosing complex structure used in your buffer. A failed +allocation returns NULL. int (*store_block)(struct zio_bi *bi, struct zio_block *block); struct zio_block (*retr_block) (struct zio_bi *bi); diff --git a/buffers/zio-buf-kmalloc.c b/buffers/zio-buf-kmalloc.c index 3331534..930ceca 100644 --- a/buffers/zio-buf-kmalloc.c +++ b/buffers/zio-buf-kmalloc.c @@ -62,11 +62,11 @@ struct zio_sysfs_operations zbk_sysfs_ops = { /* Alloc is called by the trigger (for input) or by f->write (for output) */ static struct zio_block *zbk_alloc_block(struct zio_bi *bi, - struct zio_control *ctrl, size_t datalen, gfp_t gfp) { struct zbk_instance *zbki = to_zbki(bi); struct zbk_item *item; + struct zio_control *ctrl; void *data; pr_debug("%s:%d\n", __func__, __LINE__); @@ -74,21 +74,21 @@ static struct zio_block *zbk_alloc_block(struct zio_bi *bi, /* alloc item and data. Control remains null at this point */ item = kmem_cache_alloc(zbk_slab, gfp); data = kmalloc(datalen, gfp); - if (!item || !data) + ctrl = zio_alloc_control(gfp); + if (!item || !data || !ctrl) goto out_free; memset(item, 0, sizeof(*item)); item->block.data = data; item->block.datalen = datalen; item->instance = zbki; - zio_set_ctrl(&item->block, ctrl); - return &item->block; out_free: kfree(data); kmem_cache_free(zbk_slab, item); - return ERR_PTR(-ENOMEM); + zio_free_control(ctrl); + return NULL; } /* Free is called by f->read (for input) or by the trigger (for output) */ diff --git a/buffers/zio-buf-vmalloc.c b/buffers/zio-buf-vmalloc.c index da5c15f..636eac0 100644 --- a/buffers/zio-buf-vmalloc.c +++ b/buffers/zio-buf-vmalloc.c @@ -71,11 +71,11 @@ struct zio_sysfs_operations zbk_sysfs_ops = { /* Alloc is called by the trigger (for input) or by f->write (for output) */ static struct zio_block *zbk_alloc_block(struct zio_bi *bi, - struct zio_control *ctrl, size_t datalen, gfp_t gfp) { struct zbk_instance *zbki = to_zbki(bi); struct zbk_item *item; + struct zio_control *ctrl; unsigned long offset; pr_debug("%s:%d\n", __func__, __LINE__); @@ -83,7 +83,8 @@ static struct zio_block *zbk_alloc_block(struct zio_bi *bi, /* alloc item and data. Control remains null at this point */ item = kmem_cache_alloc(zbk_slab, gfp); offset = zio_ffa_alloc(zbki->ffa, datalen, gfp); - if (!item || offset == ZIO_FFA_NOSPACE) + ctrl = zio_alloc_control(gfp); + if (!item || !ctrl || offset == ZIO_FFA_NOSPACE) goto out_free; memset(item, 0, sizeof(*item)); item->begin = offset; @@ -91,7 +92,7 @@ static struct zio_block *zbk_alloc_block(struct zio_bi *bi, item->block.data = zbki->data + offset; item->block.datalen = datalen; item->instance = zbki; - + /* mem_offset in current_ctrl is the last allocated */ bi->chan->current_ctrl->mem_offset = offset; zio_set_ctrl(&item->block, ctrl); return &item->block; @@ -99,9 +100,9 @@ static struct zio_block *zbk_alloc_block(struct zio_bi *bi, out_free: if (offset != ZIO_FFA_NOSPACE) zio_ffa_free_s(zbki->ffa, offset, datalen); - if (item) - kmem_cache_free(zbk_slab, item); - return ERR_PTR(-ENOMEM); + kmem_cache_free(zbk_slab, item); + zio_free_control(ctrl); + return NULL; } /* Free is called by f->read (for input) or by the trigger (for output) */ @@ -153,12 +154,9 @@ static int zbk_store_block(struct zio_bi *bi, struct zio_block *block) pr_debug("%s:%d (%p, %p)\n", __func__, __LINE__, bi, block); - if (unlikely(!zio_get_ctrl(block))) { - WARN_ON(1); - return -EINVAL; - } - item = to_item(block); + zio_get_ctrl(block)->mem_offset = item->begin; + output = (bi->flags & ZIO_DIR) == ZIO_DIR_OUTPUT; /* add to the buffer instance or push to the trigger */ diff --git a/doc/zio-manual.in b/doc/zio-manual.in index 449bbb4..d7c1f51 100644 --- a/doc/zio-manual.in +++ b/doc/zio-manual.in @@ -1198,7 +1198,6 @@ The buffer operations are defined as follows: @smallexample struct zio_buffer_operations { struct zio_block * (*alloc_block)(struct zio_bi *bi, - struct zio_control *ctrl, size_t datalen, gfp_t gfp); void (*free_block)(struct zio_bi *bi, struct zio_block *block); @@ -1226,7 +1225,8 @@ This is the specific role of each method in the structure: @code{create} operation. The returned @code{zio_bi} structure will usually be part of a bigger structure used internally by the buffer implementation, using the @code{container_of} - macro to access it from the @code{zio_bi} pointer. + macro to access it from the @code{zio_bi} pointer. If creation + fails, the method must return an @t{ERR_PTR} value, not NULL. @cindex alloc_block @cindex free_block @@ -1236,7 +1236,10 @@ This is the specific role of each method in the structure: The buffer is concerned with memory management, so whenever the trigger or the @i{write} system call need a new block, they ask it to the buffer type. Similarly, the buffer type - is asked to release blocks. + is asked to release blocks. The function must also allocate + the control, likely with @t{zio_alloc_control}. Such control + will be filled with the current values for the channel in due time. + (For input this copy happens late). On error it return NULL. @cindex store_block @cindex retr_block diff --git a/include/linux/zio-buffer.h b/include/linux/zio-buffer.h index 9d56267..cf56aa0 100644 --- a/include/linux/zio-buffer.h +++ b/include/linux/zio-buffer.h @@ -120,18 +120,21 @@ struct zio_block { * is usually inside a custom structure, reached by container_of(). * Thus, all blocks for a buffer type must be allocated and freed using * the methods of that specific buffer type. + * */ struct zio_buffer_operations { + /* Alloc returns NULL on error */ struct zio_block * (*alloc_block)(struct zio_bi *bi, - struct zio_control *ctrl, size_t datalen, gfp_t gfp); void (*free_block)(struct zio_bi *bi, struct zio_block *block); + /* Retr return NULL when empty */ + struct zio_block * (*retr_block) (struct zio_bi *bi); int (*store_block)(struct zio_bi *bi, struct zio_block *block); - struct zio_block * (*retr_block) (struct zio_bi *bi); + /* Create returns ERR_PTR on error */ struct zio_bi * (*create)(struct zio_buffer_type *zbuf, struct zio_channel *chan); void (*destroy)(struct zio_bi *bi); diff --git a/zio-cdev.c b/zio-cdev.c index fbe7525..c041b5e 100644 --- a/zio-cdev.c +++ b/zio-cdev.c @@ -320,21 +320,13 @@ static int __zio_read_allowed(struct zio_f_priv *priv) } /* Write is more tricky: we need control, so we may ask it to the trigger */ -static struct zio_block *__zio_write_allocblock(struct zio_bi *bi, - struct zio_control *ctrl) +static struct zio_block *__zio_write_allocblock(struct zio_bi *bi) { - struct zio_block *block; + struct zio_cset *cset = bi->chan->cset; size_t datalen; - if (!ctrl) { - ctrl = zio_alloc_control(GFP_KERNEL); - if (!ctrl) - return NULL; - memcpy(ctrl, bi->chan->current_ctrl, ZIO_CONTROL_SIZE); - } - datalen = ctrl->ssize * ctrl->nsamples; - block = bi->b_op->alloc_block(bi, ctrl, datalen, GFP_KERNEL); - return block; + datalen = cset->ssize * cset->ti->nsamples; + return bi->b_op->alloc_block(bi, datalen, GFP_KERNEL); } static int __zio_write_allowed(struct zio_f_priv *priv) @@ -351,7 +343,7 @@ static int __zio_write_allowed(struct zio_f_priv *priv) /* We want to write data. If we have no control, retrieve one */ if (!chan->user_block) - chan->user_block = __zio_write_allocblock(bi, NULL); + chan->user_block = __zio_write_allocblock(bi); block = chan->user_block; if (!block) return 0; @@ -365,7 +357,7 @@ static int __zio_write_allowed(struct zio_f_priv *priv) return 0; /* We sent it: get a new one for this new data */ - chan->user_block = __zio_write_allocblock(bi, NULL); + chan->user_block = __zio_write_allocblock(bi); return chan->user_block ? can_write : 0; } diff --git a/zio-sys.c b/zio-sys.c index 6cd30f4..ed0a4aa 100644 --- a/zio-sys.c +++ b/zio-sys.c @@ -278,8 +278,8 @@ static void __zio_fire_input_trigger(struct zio_ti *ti) struct zio_device *zdev; struct zio_cset *cset; struct zio_channel *chan; - struct zio_control *ch_ctrl, *ctrl; - int datalen, errdone = 0; + struct zio_control *ctrl; + int datalen; cset = ti->cset; zdev = cset->zdev; @@ -289,26 +289,14 @@ static void __zio_fire_input_trigger(struct zio_ti *ti) /* Allocate the buffer for the incoming sample, in active channels */ chan_for_each(chan, cset) { - ch_ctrl = chan->current_ctrl; - ch_ctrl->seq_num++; - ctrl = zio_alloc_control(GFP_ATOMIC); - if (!ctrl) { - if (!errdone++) - pr_err("%s: can't alloc control\n", __func__); - continue; - } - ch_ctrl->nsamples = ti->nsamples; - datalen = ch_ctrl->ssize * ti->nsamples; - block = zbuf->b_op->alloc_block(chan->bi, ctrl, datalen, + ctrl = chan->current_ctrl; + ctrl->seq_num++; + + ctrl->nsamples = ti->nsamples; + datalen = ctrl->ssize * ti->nsamples; + block = zbuf->b_op->alloc_block(chan->bi, datalen, GFP_ATOMIC); - if (IS_ERR(block)) { - /* Remove the following print, it's common */ - if (0 && !errdone++) - pr_err("%s: can't alloc block\n", __func__); - zio_free_control(ctrl); - chan->active_block = NULL; - continue; - } + /* on error it returns NULL so we are all happy */ chan->active_block = block; } if (!cset->raw_io(cset)) { -- GitLab