lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 22 Jul 2021 08:48:15 -0700 From: Eric Biggers <ebiggers@...nel.org> To: Chao Yu <chao@...nel.org> Cc: jaegeuk@...nel.org, Chao Yu <chao.yu@...ux.dev>, linux-kernel@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net Subject: Re: [f2fs-dev] [PATCH] f2fs: simplify accounting inflight directIO request On Thu, Jul 22, 2021 at 09:16:17PM +0800, Chao Yu wrote: > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index ba120d55e9b1..d0a1ca6ae38e 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1720,6 +1720,9 @@ static int __get_data_block(struct inode *inode, sector_t iblock, > map_bh(bh, inode->i_sb, map.m_pblk); > bh->b_state = (bh->b_state & ~F2FS_MAP_FLAGS) | map.m_flags; > bh->b_size = blks_to_bytes(inode, map.m_len); > + > + if (flag == F2FS_GET_BLOCK_DIO) > + bh->b_private = (void *)may_write; Why is this hunk needed? > +static int f2fs_dio_end_io(struct kiocb *iocb, loff_t offset, > + ssize_t bytes, void *private) > { > - struct f2fs_private_dio *dio = bio->bi_private; > - > - dec_page_count(F2FS_I_SB(dio->inode), > - dio->write ? F2FS_DIO_WRITE : F2FS_DIO_READ); > - > - bio->bi_private = dio->orig_private; > - bio->bi_end_io = dio->orig_end_io; > - > - kfree(dio); > + struct inode *inode = file_inode(iocb->ki_filp); > + bool may_write = private; > > - bio_endio(bio); > + dec_dio_req_count(F2FS_I_SB(inode), may_write ? WRITE : READ); > + return 0; > } > > static void f2fs_dio_submit_bio(struct bio *bio, struct inode *inode, > loff_t file_offset) > { > - struct f2fs_private_dio *dio; > - bool write = (bio_op(bio) == REQ_OP_WRITE); > - > - dio = f2fs_kzalloc(F2FS_I_SB(inode), > - sizeof(struct f2fs_private_dio), GFP_NOFS); > - if (!dio) > - goto out; > - > - dio->inode = inode; > - dio->orig_end_io = bio->bi_end_io; > - dio->orig_private = bio->bi_private; > - dio->write = write; > - > - bio->bi_end_io = f2fs_dio_end_io; > - bio->bi_private = dio; > - > - inc_page_count(F2FS_I_SB(inode), > - write ? F2FS_DIO_WRITE : F2FS_DIO_READ); > + inc_dio_req_count(F2FS_I_SB(inode), > + op_is_write(bio_op(bio)) ? WRITE : READ); > > submit_bio(bio); > - return; > -out: > - bio->bi_status = BLK_STS_IOERR; > - bio_endio(bio); > } The inc and dec here aren't correctly paired, since f2fs_dio_submit_bio() is called once per bio whereas f2fs_dio_end_io() is called when the entire direct I/O request (which may have consisted of multiple bios) has completed. The correct way to do this would be to do the inc before calling __blockdev_direct_IO(), and do the dec in end_io or if __blockdev_direct_IO() returned without actually issuing any I/O. But I think you shouldn't bother with this part of the change before we switch to iomap, as it will then need to be changed again anyway. - Eric
Powered by blists - more mailing lists