Skip to content

Commit 3e1842e

Browse files
chaseyuJaegeuk Kim
chaseyu
authored and
Jaegeuk Kim
committed
f2fs: fix to let caller retry allocating block address
Configure io_bits with 2 and enable LFS mode, generic/013 reports below dmesg: BUG: unable to handle kernel NULL pointer dereference at 00000104 *pdpt = 0000000029b7b001 *pde = 0000000000000000 Oops: 0002 [raspberrypi#1] PREEMPT SMP Modules linked in: crc32_generic zram f2fs(O) rfcomm bnep bluetooth ecdh_generic snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq pcbc joydev snd_seq_device aesni_intel snd_timer aes_i586 snd crypto_simd cryptd soundcore i2c_piix4 serio_raw mac_hid video parport_pc ppdev lp parport hid_generic psmouse usbhid hid e1000 CPU: 0 PID: 11161 Comm: fsstress Tainted: G O 4.17.0-rc2 raspberrypi#38 Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 EIP: f2fs_submit_page_write+0x28d/0x550 [f2fs] EFLAGS: 00010206 CPU: 0 EAX: e863dcd8 EBX: 00000000 ECX: 00000100 EDX: 00000200 ESI: e863dcf4 EDI: f6f82768 EBP: e863dbb0 ESP: e863db74 DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 CR0: 80050033 CR2: 00000104 CR3: 29a62020 CR4: 000406f0 Call Trace: do_write_page+0x6f/0xc0 [f2fs] write_data_page+0x4a/0xd0 [f2fs] do_write_data_page+0x327/0x630 [f2fs] __write_data_page+0x34b/0x820 [f2fs] __f2fs_write_data_pages+0x42d/0x8c0 [f2fs] f2fs_write_data_pages+0x27/0x30 [f2fs] do_writepages+0x1a/0x70 __filemap_fdatawrite_range+0x94/0xd0 filemap_write_and_wait_range+0x3d/0xa0 __generic_file_write_iter+0x11a/0x1f0 f2fs_file_write_iter+0xdd/0x3b0 [f2fs] __vfs_write+0xd2/0x150 vfs_write+0x9b/0x190 ksys_write+0x45/0x90 sys_write+0x16/0x20 do_fast_syscall_32+0xaa/0x22c entry_SYSENTER_32+0x4c/0x7b EIP: 0xb7fc8c51 EFLAGS: 00000246 CPU: 0 EAX: ffffffda EBX: 00000003 ECX: 09cde000 EDX: 00001000 ESI: 00000003 EDI: 00001000 EBP: 00000000 ESP: bfbded38 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b Code: e8 f9 77 34 c9 8b 45 e0 8b 80 b8 00 00 00 39 45 d8 0f 84 bb 02 00 00 8b 45 e0 8b 80 b8 00 00 00 8d 50 d8 8b 08 89 55 f0 8b 50 04 <89> 51 04 89 0a c7 00 00 01 00 00 c7 40 04 00 02 00 00 8b 45 dc EIP: f2fs_submit_page_write+0x28d/0x550 [f2fs] SS:ESP: 0068:e863db74 CR2: 0000000000000104 ---[ end trace 4cac79c0d1305ee6 ]--- allocate_data_block will submit all sequential pending IOs sorted by a FIFO list, If we failed to submit other user's IO due to unaligned write, we will retry to allocate new block address for current IO, then it will initialize fio.list again, if fio was in the list before, it can break FIFO list, result in above panic. Thread A Thread B - do_write_page - allocate_data_block - list_add_tail : fioA cached in FIFO list. - do_write_page - allocate_data_block - list_add_tail : fioB cached in FIFO list. - f2fs_submit_page_write : fail to submit IO - allocate_data_block - INIT_LIST_HEAD - f2fs_submit_page_write - list_del <-- NULL pointer dereference This patch adds fio.retry parameter to indicate failure status for each IO, and avoid bailing out if there is still pending IO in FIFO list for fixing. Signed-off-by: Chao Yu <[email protected]> Signed-off-by: Jaegeuk Kim <[email protected]>
1 parent b97a9b9 commit 3e1842e

File tree

4 files changed

+17
-16
lines changed

4 files changed

+17
-16
lines changed

‎fs/f2fs/data.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -462,13 +462,12 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
462462
return 0;
463463
}
464464

465-
int f2fs_submit_page_write(struct f2fs_io_info *fio)
465+
void f2fs_submit_page_write(struct f2fs_io_info *fio)
466466
{
467467
struct f2fs_sb_info *sbi = fio->sbi;
468468
enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
469469
struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
470470
struct page *bio_page;
471-
int err = 0;
472471

473472
f2fs_bug_on(sbi, is_read_io(fio->op));
474473

@@ -478,7 +477,7 @@ int f2fs_submit_page_write(struct f2fs_io_info *fio)
478477
spin_lock(&io->io_lock);
479478
if (list_empty(&io->io_list)) {
480479
spin_unlock(&io->io_lock);
481-
goto out_fail;
480+
goto out;
482481
}
483482
fio = list_first_entry(&io->io_list,
484483
struct f2fs_io_info, list);
@@ -505,9 +504,9 @@ int f2fs_submit_page_write(struct f2fs_io_info *fio)
505504
if (io->bio == NULL) {
506505
if ((fio->type == DATA || fio->type == NODE) &&
507506
fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
508-
err = -EAGAIN;
509507
dec_page_count(sbi, WB_DATA_TYPE(bio_page));
510-
goto out_fail;
508+
fio->retry = true;
509+
goto skip;
511510
}
512511
io->bio = __bio_alloc(sbi, fio->new_blkaddr, fio->io_wbc,
513512
BIO_MAX_PAGES, false,
@@ -527,12 +526,11 @@ int f2fs_submit_page_write(struct f2fs_io_info *fio)
527526
f2fs_trace_ios(fio, 0);
528527

529528
trace_f2fs_submit_page_write(fio->page, fio);
530-
529+
skip:
531530
if (fio->in_list)
532531
goto next;
533-
out_fail:
532+
out:
534533
up_write(&io->io_rwsem);
535-
return err;
536534
}
537535

538536
static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,

‎fs/f2fs/f2fs.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1012,6 +1012,7 @@ struct f2fs_io_info {
10121012
int need_lock; /* indicate we need to lock cp_rwsem */
10131013
bool in_list; /* indicate fio is in io_list */
10141014
bool is_meta; /* indicate borrow meta inode mapping or not */
1015+
bool retry; /* need to reallocate block address */
10151016
enum iostat_type io_type; /* io type */
10161017
struct writeback_control *io_wbc; /* writeback control */
10171018
};
@@ -2929,7 +2930,7 @@ void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
29292930
enum page_type type);
29302931
void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
29312932
int f2fs_submit_page_bio(struct f2fs_io_info *fio);
2932-
int f2fs_submit_page_write(struct f2fs_io_info *fio);
2933+
void f2fs_submit_page_write(struct f2fs_io_info *fio);
29332934
struct block_device *f2fs_target_device(struct f2fs_sb_info *sbi,
29342935
block_t blk_addr, struct bio *bio);
29352936
int f2fs_target_device_index(struct f2fs_sb_info *sbi, block_t blkaddr);

‎fs/f2fs/gc.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,7 @@ static void move_data_block(struct inode *inode, block_t bidx,
603603
.op_flags = 0,
604604
.encrypted_page = NULL,
605605
.in_list = false,
606+
.retry = false,
606607
};
607608
struct dnode_of_data dn;
608609
struct f2fs_summary sum;
@@ -697,8 +698,8 @@ static void move_data_block(struct inode *inode, block_t bidx,
697698
fio.op = REQ_OP_WRITE;
698699
fio.op_flags = REQ_SYNC;
699700
fio.new_blkaddr = newaddr;
700-
err = f2fs_submit_page_write(&fio);
701-
if (err) {
701+
f2fs_submit_page_write(&fio);
702+
if (fio.retry) {
702703
if (PageWriteback(fio.encrypted_page))
703704
end_page_writeback(fio.encrypted_page);
704705
goto put_page_out;

‎fs/f2fs/segment.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2731,6 +2731,7 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
27312731

27322732
INIT_LIST_HEAD(&fio->list);
27332733
fio->in_list = true;
2734+
fio->retry = false;
27342735
io = sbi->write_io[fio->type] + fio->temp;
27352736
spin_lock(&io->io_lock);
27362737
list_add_tail(&fio->list, &io->io_list);
@@ -2766,7 +2767,6 @@ static void update_device_state(struct f2fs_io_info *fio)
27662767
static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
27672768
{
27682769
int type = __get_segment_type(fio);
2769-
int err;
27702770
bool keep_order = (test_opt(fio->sbi, LFS) && type == CURSEG_COLD_DATA);
27712771

27722772
if (keep_order)
@@ -2776,13 +2776,14 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
27762776
&fio->new_blkaddr, sum, type, fio, true);
27772777

27782778
/* writeout dirty page into bdev */
2779-
err = f2fs_submit_page_write(fio);
2780-
if (err == -EAGAIN) {
2779+
f2fs_submit_page_write(fio);
2780+
if (fio->retry) {
27812781
fio->old_blkaddr = fio->new_blkaddr;
27822782
goto reallocate;
2783-
} else if (!err) {
2784-
update_device_state(fio);
27852783
}
2784+
2785+
update_device_state(fio);
2786+
27862787
if (keep_order)
27872788
up_read(&fio->sbi->io_order_lock);
27882789
}

0 commit comments

Comments
 (0)