Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

jffs2: GC deadlock reading a page that is used in jffs2_write_begin()

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>

authored by

Kyeong Yoo and committed by
Richard Weinberger
aa39cc67 50cb4373

+25 -15
+25 -15
fs/jffs2/file.c
··· 136 136 struct page *pg; 137 137 struct inode *inode = mapping->host; 138 138 struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); 139 + struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); 139 140 pgoff_t index = pos >> PAGE_SHIFT; 140 141 uint32_t pageofs = index << PAGE_SHIFT; 141 142 int ret = 0; 142 - 143 - pg = grab_cache_page_write_begin(mapping, index, flags); 144 - if (!pg) 145 - return -ENOMEM; 146 - *pagep = pg; 147 143 148 144 jffs2_dbg(1, "%s()\n", __func__); 149 145 150 146 if (pageofs > inode->i_size) { 151 147 /* Make new hole frag from old EOF to new page */ 152 - struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); 153 148 struct jffs2_raw_inode ri; 154 149 struct jffs2_full_dnode *fn; 155 150 uint32_t alloc_len; ··· 155 160 ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, 156 161 ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); 157 162 if (ret) 158 - goto out_page; 163 + goto out_err; 159 164 160 165 mutex_lock(&f->sem); 161 166 memset(&ri, 0, sizeof(ri)); ··· 185 190 ret = PTR_ERR(fn); 186 191 jffs2_complete_reservation(c); 187 192 mutex_unlock(&f->sem); 188 - goto out_page; 193 + goto out_err; 189 194 } 190 195 ret = jffs2_add_full_dnode_to_inode(c, f, fn); 191 196 if (f->metadata) { ··· 200 205 jffs2_free_full_dnode(fn); 201 206 jffs2_complete_reservation(c); 202 207 mutex_unlock(&f->sem); 203 - goto out_page; 208 + goto out_err; 204 209 } 205 210 jffs2_complete_reservation(c); 206 211 inode->i_size = pageofs; 207 212 mutex_unlock(&f->sem); 208 213 } 214 + 215 + /* 216 + * While getting a page and reading data in, lock c->alloc_sem until 217 + * the page is Uptodate. Otherwise GC task may attempt to read the same 218 + * page in read_cache_page(), which causes a deadlock. 219 + */ 220 + mutex_lock(&c->alloc_sem); 221 + pg = grab_cache_page_write_begin(mapping, index, flags); 222 + if (!pg) { 223 + ret = -ENOMEM; 224 + goto release_sem; 225 + } 226 + *pagep = pg; 209 227 210 228 /* 211 229 * Read in the page if it wasn't already present. Cannot optimize away ··· 229 221 mutex_lock(&f->sem); 230 222 ret = jffs2_do_readpage_nolock(inode, pg); 231 223 mutex_unlock(&f->sem); 232 - if (ret) 233 - goto out_page; 224 + if (ret) { 225 + unlock_page(pg); 226 + put_page(pg); 227 + goto release_sem; 228 + } 234 229 } 235 230 jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags); 236 - return ret; 237 231 238 - out_page: 239 - unlock_page(pg); 240 - put_page(pg); 232 + release_sem: 233 + mutex_unlock(&c->alloc_sem); 234 + out_err: 241 235 return ret; 242 236 } 243 237