On Sat, May 23, 2020 at 02:34:51AM -0700, Christoph Hellwig wrote:
> > --- a/fs/xfs/xfs_trans_priv.h
> > +++ b/fs/xfs/xfs_trans_priv.h
> > @@ -143,15 +143,10 @@ static inline void
> > xfs_clear_li_failed(
> > struct xfs_log_item *lip)
> > {
> > - struct xfs_buf *bp = lip->li_buf;
> > -
> > ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags));
> > lockdep_assert_held(&lip->li_ailp->ail_lock);
> >
> > - if (test_and_clear_bit(XFS_LI_FAILED, &lip->li_flags)) {
> > - lip->li_buf = NULL;
> > - xfs_buf_rele(bp);
> > - }
> > + clear_bit(XFS_LI_FAILED, &lip->li_flags);
> > }
> >
> > static inline void
> > @@ -161,10 +156,7 @@ xfs_set_li_failed(
> > {
> > lockdep_assert_held(&lip->li_ailp->ail_lock);
> >
> > - if (!test_and_set_bit(XFS_LI_FAILED, &lip->li_flags)) {
> > - xfs_buf_hold(bp);
> > - lip->li_buf = bp;
> > - }
> > + set_bit(XFS_LI_FAILED, &lip->li_flags);
> > }
>
> Isn't this going to break quotas, which don't always have li_buf set?
Yup, that'll be the assert fail that Darrick reported. Well spotted,
Christoph!
I've got to rework the error handling code anyway, so I might end up
getting rid of ->li_error and hard coding these like I've done the
iodone functions. That way the different objects can use different
failure mechanisms until the dquot code is converted to the same
"hold at dirty time" flushing mechanism...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
