Re: [PATCH v2 2/2] ext4: protect ext4_release_dquot against freezing

Baokun Li
2024-12-03T08:29:35Z

On 2024/11/27 14:01, Ojaswin Mujoo wrote:
> On Tue, Nov 26, 2024 at 10:49:14PM +0800, Baokun Li wrote:
>> On 2024/11/21 20:38, Ojaswin Mujoo wrote:
>>> Protect ext4_release_dquot against freezing so that we
>>> don't try to start a transaction when FS is frozen, leading
>>> to warnings.
>>>
>>> Further, avoid taking the freeze protection if a transaction
>>> is already running so that we don't need end up in a deadlock
>>> as described in
>>>
>>> 46e294efc355 ext4: fix deadlock with fs freezing and EA inodes
>>>
>>> Suggested-by: Jan Kara <jack@suse.cz>
>>> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>>> ---
>>> fs/ext4/super.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>>> index 16a4ce704460..f7437a592359 100644
>>> --- a/fs/ext4/super.c
>>> +++ b/fs/ext4/super.c
>>> @@ -6887,12 +6887,25 @@ static int ext4_release_dquot(struct dquot *dquot)
>>> {
>>> int ret, err;
>>> handle_t *handle;
>>> + bool freeze_protected = false;
>>> +
>>> + /*
>>> + * Trying to sb_start_intwrite() in a running transaction
>>> + * can result in a deadlock. Further, running transactions
>>> + * are already protected from freezing.
>>> + */
>>> + if (!ext4_journal_current_handle()) {
>>> + sb_start_intwrite(dquot->dq_sb);
>>> + freeze_protected = true;
>>> + }
>>> handle = ext4_journal_start(dquot_to_inode(dquot), EXT4_HT_QUOTA,
>>> EXT4_QUOTA_DEL_BLOCKS(dquot->dq_sb));
>>> if (IS_ERR(handle)) {
>>> /* Release dquot anyway to avoid endless cycle in dqput() */
>>> dquot_release(dquot);
>>> + if (freeze_protected)
>>> + sb_end_intwrite(dquot->dq_sb);
>>> return PTR_ERR(handle);
>>> }
>>> ret = dquot_release(dquot);
>>> @@ -6903,6 +6916,10 @@ static int ext4_release_dquot(struct dquot *dquot)
>> The `git am` command looks for the following context code from line 6903
>> to apply the changes. But there are many functions in fs/ext4/super.c that
>> have similar code, such as ext4_write_dquot() and ext4_acquire_dquot().
> Oh that's strange, shouldn't it match the complete line like:
A rough look at the `git am` source code looks like it only focuses on
line numbers between two ‘@@’.

am_run
run_apply
apply_all_patches
apply_patch
parse_chunk
find_header
parse_fragment_header
check_patch_list
check_patch
apply_data
load_preimage
apply_fragments
apply_one_fragment
find_pos
match_fragment
>>> @@ -6903,6 +6916,10 @@ static int ext4_release_dquot(struct dquot *dquot)
> That should only have one occurence around line 6903? Or does it try to
> fuzzy match which ends up matching ext4_write_dquot etc?
In find_pos(), start from line 6903, compare the hash value of each line
of code line by line in forward direction, if it can't match, then match
the hash value of each line of code line by line in reverse direction from
line 6903. Fuzzy matching is used if some whitespace characters should be
ignored.

Regards,
Baokun