lkml.org 
[lkml]   [2022]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH AUTOSEL 4.14 42/56] jffs2: GC deadlock reading a page that is used in jffs2_write_begin()
    Date
    From: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>

    [ Upstream commit aa39cc675799bc92da153af9a13d6f969c348e82 ]

    GC task can deadlock in read_cache_page() because it may attempt
    to release a page that is actually allocated by another task in
    jffs2_write_begin().
    The reason is that in jffs2_write_begin() there is a small window
    a cache page is allocated for use but not set Uptodate yet.

    This ends up with a deadlock between two tasks:
    1) A task (e.g. file copy)
    - jffs2_write_begin() locks a cache page
    - jffs2_write_end() tries to lock "alloc_sem" from
    jffs2_reserve_space() <-- STUCK
    2) GC task (jffs2_gcd_mtd3)
    - jffs2_garbage_collect_pass() locks "alloc_sem"
    - try to lock the same cache page in read_cache_page() <-- STUCK

    So to avoid this deadlock, hold "alloc_sem" in jffs2_write_begin()
    while reading data in a cache page.

    Signed-off-by: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
    Signed-off-by: Richard Weinberger <richard@nod.at>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    fs/jffs2/file.c | 40 +++++++++++++++++++++++++---------------
    1 file changed, 25 insertions(+), 15 deletions(-)

    diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
    index bd0428bebe9b7..221eb2bd205e4 100644
    --- a/fs/jffs2/file.c
    +++ b/fs/jffs2/file.c
    @@ -135,20 +135,15 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
    struct page *pg;
    struct inode *inode = mapping->host;
    struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
    + struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
    pgoff_t index = pos >> PAGE_SHIFT;
    uint32_t pageofs = index << PAGE_SHIFT;
    int ret = 0;

    - pg = grab_cache_page_write_begin(mapping, index, flags);
    - if (!pg)
    - return -ENOMEM;
    - *pagep = pg;
    -
    jffs2_dbg(1, "%s()\n", __func__);

    if (pageofs > inode->i_size) {
    /* Make new hole frag from old EOF to new page */
    - struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
    struct jffs2_raw_inode ri;
    struct jffs2_full_dnode *fn;
    uint32_t alloc_len;
    @@ -159,7 +154,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
    ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
    ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
    if (ret)
    - goto out_page;
    + goto out_err;

    mutex_lock(&f->sem);
    memset(&ri, 0, sizeof(ri));
    @@ -189,7 +184,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
    ret = PTR_ERR(fn);
    jffs2_complete_reservation(c);
    mutex_unlock(&f->sem);
    - goto out_page;
    + goto out_err;
    }
    ret = jffs2_add_full_dnode_to_inode(c, f, fn);
    if (f->metadata) {
    @@ -204,13 +199,26 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
    jffs2_free_full_dnode(fn);
    jffs2_complete_reservation(c);
    mutex_unlock(&f->sem);
    - goto out_page;
    + goto out_err;
    }
    jffs2_complete_reservation(c);
    inode->i_size = pageofs;
    mutex_unlock(&f->sem);
    }

    + /*
    + * While getting a page and reading data in, lock c->alloc_sem until
    + * the page is Uptodate. Otherwise GC task may attempt to read the same
    + * page in read_cache_page(), which causes a deadlock.
    + */
    + mutex_lock(&c->alloc_sem);
    + pg = grab_cache_page_write_begin(mapping, index, flags);
    + if (!pg) {
    + ret = -ENOMEM;
    + goto release_sem;
    + }
    + *pagep = pg;
    +
    /*
    * Read in the page if it wasn't already present. Cannot optimize away
    * the whole page write case until jffs2_write_end can handle the
    @@ -220,15 +228,17 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
    mutex_lock(&f->sem);
    ret = jffs2_do_readpage_nolock(inode, pg);
    mutex_unlock(&f->sem);
    - if (ret)
    - goto out_page;
    + if (ret) {
    + unlock_page(pg);
    + put_page(pg);
    + goto release_sem;
    + }
    }
    jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags);
    - return ret;

    -out_page:
    - unlock_page(pg);
    - put_page(pg);
    +release_sem:
    + mutex_unlock(&c->alloc_sem);
    +out_err:
    return ret;
    }

    --
    2.34.1
    \
     
     \ /
      Last update: 2022-01-18 04:13    [W:4.139 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site