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: Wed, 31 May 2017 12:12:40 +0200 From: Jan Kara <jack@...e.cz> To: Daeho Jeong <daeho.jeong@...sung.com> Cc: Jan Kara <jack@...e.cz>, "jack@...e.com" <jack@...e.com>, "hch@...radead.org" <hch@...radead.org>, "tytso@....edu" <tytso@....edu>, "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org> Subject: Re: RE: Re: [PATCH] ext4: change sequential discard handling on commit complete phase into parallel manner On Wed 31-05-17 02:22:40, Daeho Jeong wrote: > Hi Jan, > > > Hi Jan, > > > > Hum, these games with several callbacks, lists, etc. look awkward and > > > unnecessary. It think they mostly come from the fact that we call separate > > > freeing callback for each extent to free which doesn't fit the needs of > > > async discard well. > > > > So instead of adding post_cb_list and several callback functions, it would > > > seem easier to have just one callback structure instead of one for every > > > extent. Then the structure would contain a list of extents that need to be > > > freed freed. So something like: > > > > struct ext4_free_data { > > > struct ext4_journal_cb_entry efd_jce; > > > struct list_head efd_extents; > > > } > > > > struct ext4_freed_extent { > > > struct list_head efe_list; > > > struct rb_node efe_node; > > > ext4_group_t efe_group; > > > ext4_grpblk_t efe_start_cluster; > > > ext4_grpblk_t efe_count; > > > tid_t efe_tid; > > > } > > > > When commit happens, we can just walk the efd_extents list while efe_tid is > > > equal tid of the transaction for which the callback was called and submit all > > > discard requests. You can use bio chaining implemented in > > > __blkdev_issue_discard() which XFS already uses and so the result of all > > > the discards you submit will be just one bio. Then you walk the list of > > > extents again and free them in the buddy bitmaps. And finally, you wait for > > > the bio to complete. All will be then happening in one function and it will > > > be much easier to understand. > > > It's right. the patch didn't look neat because of a few callbacks and the > > post callback list. I will modify the patch as your suggestion. It will > > look better. > > > Thank you very much. :-) > > It's a little difficult to decide when we have to add new ext4_free_data > entry for a transaction for the first time and how do we know whether the > ext4_free_data entry for a transaction is already added or not? I think > that it is a bad idea to search in t_private_list of the transaction for > that, because there might be the different type of callback entries in > the future. > > And how do we find the exact ext4_free_data entry for a newly created > ext4_freed_extent? We only know which transcation is related to the > ext4_freed_extent, so we could use this but I don't have any good idea > for that. Right, so looking at ext4_journal_commit_callback() it will be probably the easiest to just call some new function ext4_process_freed_extents(sb) directly from there (i.e., avoid the ext4 callback infrastructure altogether) and anchor the list of extents in the superblock. Honza -- Jan Kara <jack@...e.com> SUSE Labs, CR
Powered by blists - more mailing lists