linux-ext4 - Re: [PATCH v7 2/9] ext4: for committing inode, make ext4_fc_track

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241213151029.qeuctijrbibqoqp5@quack3>
Date: Fri, 13 Dec 2024 16:10:29 +0100
From: Jan Kara <jack@...e.cz>
To: Harshad Shirwadkar <harshadshirwadkar@...il.com>
Cc: linux-ext4@...r.kernel.org, tytso@....edu, jack@...e.cz,
	harshads@...gle.com
Subject: Re: [PATCH v7 2/9] ext4: for committing inode, make
 ext4_fc_track_inode wait

On Thu 12-12-24 23:00:43, Jan Kara wrote:
> On Sun 18-08-24 04:03:49, Harshad Shirwadkar wrote:
> > +
> > +	while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
> > +#if (BITS_PER_LONG < 64)
> > +		DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
> > +				EXT4_STATE_FC_COMMITTING);
> > +		wq = bit_waitqueue(&ei->i_state_flags,
> > +				   EXT4_STATE_FC_COMMITTING);
> > +#else
> > +		DEFINE_WAIT_BIT(wait, &ei->i_flags,
> > +				EXT4_STATE_FC_COMMITTING);
> > +		wq = bit_waitqueue(&ei->i_flags,
> > +				   EXT4_STATE_FC_COMMITTING);
> > +#endif
> > +		prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
> > +		if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
> > +			schedule();
> > +		finish_wait(wq, &wait.wq_entry);
> > +	}
> 
> But what protects us from fastcommit setting EXT4_STATE_FC_COMMITTING at
> this moment before we call ext4_fc_track_template(). Don't you need
> to grab sbi->s_fc_lock and hold it until the inode is attached to the
> fastcommit?
> 
> I might be missing something so some documentation (like a comment here)
> would be nice to explain what are you actually trying to achieve with the
> waiting...

Ah, I see now. In patch 4 you make setting of EXT4_STATE_FC_COMMITTING
protected by journal locking so holding a handle open protects us. Good.
But please comment about it in the changelog.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR