[bug report] f2fs: fix missing unlock(sbi->gc_mutex)

[bug report] f2fs: fix missing unlock(sbi->gc_mutex)

[Dan Carpenter]

Hello Jaegeuk Kim,

The patch 8f31b4665c14: "f2fs: fix missing unlock(sbi->gc_mutex)"
from Dec 17, 2018, leads to the following static checker warning:

	fs/f2fs/super.c:1481 f2fs_disable_checkpoint()
	error: double lock 'mutex:&sbi->gc_mutex'

fs/f2fs/super.c
    1456 static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
    1457 {
    1458 	struct cp_control cpc;
    1459 	int err;
    1460 
    1461 	sbi->sb->s_flags |= SB_ACTIVE;
    1462 
    1463 	f2fs_update_time(sbi, DISABLE_TIME);
    1464 
    1465 	while (!f2fs_time_over(sbi, DISABLE_TIME)) {
    1466 		mutex_lock(&sbi->gc_mutex);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
Lock.

    1467 		err = f2fs_gc(sbi, true, false, NULL_SEGNO);
    1468 		if (err == -ENODATA)
    1469 			break;
    1470 		if (err && err != -EAGAIN)
    1471 			return err;
    1472 	}
    1473 
    1474 	err = sync_filesystem(sbi->sb);
    1475 	if (err)
    1476 		return err;
    1477 
    1478 	if (f2fs_disable_cp_again(sbi))
    1479 		return -EAGAIN;
    1480 
--> 1481 	mutex_lock(&sbi->gc_mutex);
                            ^^^^^^^^^^^^^
Deadlock?

    1482 	cpc.reason = CP_PAUSE;
    1483 	set_sbi_flag(sbi, SBI_CP_DISABLED);
    1484 	f2fs_write_checkpoint(sbi, &cpc);
    1485 
    1486 	sbi->unusable_block_count = 0;
    1487 	mutex_unlock(&sbi->gc_mutex);
    1488 	return 0;
    1489 }

regards,
dan carpenter

Re: [bug report] f2fs: fix missing unlock(sbi->gc_mutex)

[Chao Yu]

Hi Dan,

Thanks for the report.

The code logic here is okay, since unlock will be called inside f2fs_gc()
anyway. :)

FYI, the same report in below link from Julia, I've replied once. :)

https://lists.01.org/pipermail/kbuild-all/2018-December/056436.html

Thanks,

On 2019/1/3 18:54, Dan Carpenter wrote:
> Hello Jaegeuk Kim,
> 
> The patch 8f31b4665c14: "f2fs: fix missing unlock(sbi->gc_mutex)"
> from Dec 17, 2018, leads to the following static checker warning:
> 
> 	fs/f2fs/super.c:1481 f2fs_disable_checkpoint()
> 	error: double lock 'mutex:&sbi->gc_mutex'
> 
> fs/f2fs/super.c
>     1456 static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
>     1457 {
>     1458 	struct cp_control cpc;
>     1459 	int err;
>     1460 
>     1461 	sbi->sb->s_flags |= SB_ACTIVE;
>     1462 
>     1463 	f2fs_update_time(sbi, DISABLE_TIME);
>     1464 
>     1465 	while (!f2fs_time_over(sbi, DISABLE_TIME)) {
>     1466 		mutex_lock(&sbi->gc_mutex);
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
> Lock.
> 
>     1467 		err = f2fs_gc(sbi, true, false, NULL_SEGNO);
>     1468 		if (err == -ENODATA)
>     1469 			break;
>     1470 		if (err && err != -EAGAIN)
>     1471 			return err;
>     1472 	}
>     1473 
>     1474 	err = sync_filesystem(sbi->sb);
>     1475 	if (err)
>     1476 		return err;
>     1477 
>     1478 	if (f2fs_disable_cp_again(sbi))
>     1479 		return -EAGAIN;
>     1480 
> --> 1481 	mutex_lock(&sbi->gc_mutex);
>                             ^^^^^^^^^^^^^
> Deadlock?
> 
>     1482 	cpc.reason = CP_PAUSE;
>     1483 	set_sbi_flag(sbi, SBI_CP_DISABLED);
>     1484 	f2fs_write_checkpoint(sbi, &cpc);
>     1485 
>     1486 	sbi->unusable_block_count = 0;
>     1487 	mutex_unlock(&sbi->gc_mutex);
>     1488 	return 0;
>     1489 }
> 
> regards,
> dan carpenter
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
>