Hi Javier,
one small issue I found for error path while going through changes:
On Mon, Jun 26, 2017 at 11:57:17AM +0200, Javier González wrote:
..
> +static int pblk_lines_alloc_metadata(struct pblk *pblk)
> +{
> + struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> + struct pblk_line_meta *lm = &pblk->lm;
> + int i;
> +
> + /* smeta is always small enough to fit on a kmalloc memory allocation,
> + * emeta depends on the number of LUNs allocated to the pblk instance
> + */
> + l_mg->smeta_alloc_type = PBLK_KMALLOC_META;
> + for (i = 0; i < PBLK_DATA_LINES; i++) {
> + l_mg->sline_meta[i] = kmalloc(lm->smeta_len, GFP_KERNEL);
> + if (!l_mg->sline_meta[i])
> + goto fail_free_smeta;
For this error path the the goto label at end doesn't free up
resources correctly. It needs a
while (--index >= 0)...
logic with appropriate adjustment.
> + }
> +
> + /* emeta allocates three different buffers for managing metadata with
> + * in-memory and in-media layouts
> + */
> + for (i = 0; i < PBLK_DATA_LINES; i++) {
> + struct pblk_emeta *emeta;
> +
> + emeta = kmalloc(sizeof(struct pblk_emeta), GFP_KERNEL);
> + if (!emeta)
> + goto fail_free_emeta;
> +
> + if (lm->emeta_len[0] > KMALLOC_MAX_CACHE_SIZE) {
> + l_mg->emeta_alloc_type = PBLK_VMALLOC_META;
> +
> + emeta->buf = vmalloc(lm->emeta_len[0]);
> + if (!emeta->buf) {
> + kfree(emeta);
> + goto fail_free_emeta;
> + }
> +
> + emeta->nr_entries = lm->emeta_sec[0];
> + l_mg->eline_meta[i] = emeta;
> + } else {
> + l_mg->emeta_alloc_type = PBLK_KMALLOC_META;
> +
> + emeta->buf = kmalloc(lm->emeta_len[0], GFP_KERNEL);
> + if (!emeta->buf) {
> + kfree(emeta);
> + goto fail_free_emeta;
> + }
> +
> + emeta->nr_entries = lm->emeta_sec[0];
> + l_mg->eline_meta[i] = emeta;
> + }
> + }
> +
> + l_mg->vsc_list = kcalloc(l_mg->nr_lines, sizeof(__le32), GFP_KERNEL);
> + if (!l_mg->vsc_list)
> + goto fail_free_emeta;
> +
> + for (i = 0; i < l_mg->nr_lines; i++)
> + l_mg->vsc_list[i] = cpu_to_le32(EMPTY_ENTRY);
> +
> + return 0;
> +
> +fail_free_emeta:
> + while (--i >= 0) {
> + vfree(l_mg->eline_meta[i]->buf);
This would need l_mg->emeta_alloc_type check to decide whether
allocation was done with kmalloc or vmalloc.
> + kfree(&l_mg->eline_meta[i]);
> + }
> +
> +fail_free_smeta:
> + for (i = 0; i < PBLK_DATA_LINES; i++)
> + pblk_mfree(&l_mg->sline_meta[i], l_mg->smeta_alloc_type);
> +
> + return -ENOMEM;
> +}
> +
Thanks,
