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@xxxxxxxx>
SUSE Labs, CR
