Re: [PATCH] lightnvm: pblk: adjust the position of the lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/27/2018 03:24 PM, Javier Gonzalez wrote:

On 27 Nov 2018, at 15.22, Matias Bjørling <mb@xxxxxxxxxxx> wrote:

On 11/27/2018 01:57 PM, Javier Gonzalez wrote:
On 27 Nov 2018, at 02.53, Hua Su <suhua.tanke@xxxxxxxxx> wrote:

Add lock protection for list operations.
Signed-off-by: Hua Su <suhua.tanke@xxxxxxxxx>
---
drivers/lightnvm/pblk-core.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 6944aac43b01..e490df217dac 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1286,24 +1286,27 @@ int pblk_line_recov_alloc(struct pblk *pblk, struct pblk_line *line)
    list_del(&line->list);

    ret = pblk_line_prepare(pblk, line);
-    if (ret) {
-        list_add(&line->list, &l_mg->free_list);
-        spin_unlock(&l_mg->free_lock);
-        return ret;
-    }
-    spin_unlock(&l_mg->free_lock);
+    if (ret)
+        goto out;

    ret = pblk_line_alloc_bitmaps(pblk, line);
    if (ret)
-        return ret;
+        goto out;

    if (!pblk_line_init_bb(pblk, line, 0)) {
        list_add(&line->list, &l_mg->free_list);
+        spin_unlock(&l_mg->free_lock);
        return -EINTR;
    }
+    spin_unlock(&l_mg->free_lock);

    pblk_rl_free_lines_dec(&pblk->rl, line, true);
    return 0;
+
+out:
+    list_add(&line->list, &l_mg->free_list);
+    spin_unlock(&l_mg->free_lock);
+    return ret;
}

void pblk_line_recov_close(struct pblk *pblk, struct pblk_line *line)
--
2.19.1
This path is only touched by the recovery path, which is single
threaded, so there is no race condition as is. Also, if recovery fails,
pblk will not create the instance at all. This said, it would be
good to protect the list_add on the pblk_line_init_bb() error path in
case this code is used for some other purpose in the future.

I like your explanation here. Another option is that we could add a comment to notify the developer that it safe in this context?

Sure. Do you want to add it? Or should I send it?

Javier


Please send it. Thanks!



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux