Re: 答复: [PATCH] fs-writeback: writeback_sb_inodes: Do not increase 'total_wrote' when nothing is written

[Chunhai Guo]

在 2023/9/14 14:58, Jan Kara 写道:
> On Thu 14-09-23 04:12:31, 郭纯海 wrote:
> > > On Wed 13-09-23 07:15:01, Chunhai Guo wrote:
> > > >  From the dump info, there are only two pages as shown below. One is
> > > > updated and another is under writeback. Maybe f2fs counts the
> > > > writeback page as a dirty one, so get_dirty_pages() got one. As you
> > > > said, maybe this is unreasonable.
> > > >
> > > > Jaegeuk & Chao, what do you think of this?
> > > >
> > > >
> > > > crash_32> files -p 0xE5A44678
> > > >   INODE    NRPAGES
> > > > e5a44678        2
> > > >
> > > >    PAGE    PHYSICAL   MAPPING    INDEX CNT FLAGS
> > > > e8d0e338  641de000  e5a44810         0  5 a095
> > > locked,waiters,uptodate,lru,private,writeback
> > > > e8ad59a0  54528000  e5a44810         1  2 2036
> > > referenced,uptodate,lru,active,private
> > >
> > > Indeed, incrementing pages_skipped when there's no dirty page is a bit odd.
> > > That being said we could also harden requeue_inode() - in particular we could do
> > > there:
> > >
> > >          if (wbc->pages_skipped) {
> > >                  /*
> > >                   * Writeback is not making progress due to locked buffers.
> > >                   * Skip this inode for now. Although having skipped pages
> > >                   * is odd for clean inodes, it can happen for some
> > >                   * filesystems so handle that gracefully.
> > >                   */
> > >                  if (inode->i_state & I_DIRTY_ALL)
> > >                          redirty_tail_locked(inode, wb);
> > >                  else
> > >                          inode_cgwb_move_to_attached(inode, wb);
> > >          }
> > >
> > > Does this fix your problem as well?
> > >
> > >                                                                  Honza
> >
> > Thank you for your reply. Did you forget the 'return' statement? Since I encountered this issue on the 4.19 kernel and there is not inode_cgwb_move_to_attached() yet, I replaced it with inode_io_list_del_locked(). So, below is the test patch I am applying. Please have a check. By the way, the test will take some time. I will provide feedback when it is finished. Thanks.
>
> Yeah, I forgot about the return.

Hi Jan,
The test is finished and this patch can fix this issue, too.
Thanks,
> > 	if (wbc->pages_skipped) {
> > 		/*
> > 		 * writeback is not making progress due to locked
> > 		 * buffers. Skip this inode for now.
> > 		 */
> > -		redirty_tail_locked(inode, wb);
> > +		if (inode->i_state & I_DIRTY_ALL)
> > +			redirty_tail_locked(inode, wb);
> > +		else
> > +			inode_io_list_del_locked(inode, wb);
> >   		return;
> >   	}
>
> Looks good. Thanks for testing!
>
> 								Honza