Re: [PATCH] ext4: add a configurable parameter to prevent endless loop in ext4_mb_discard_group_p

[Jan Kara]

On Fri 09-04-21 11:17:33, riteshh wrote:
> On 21/04/09 02:50AM, Wen Yang wrote:
> > > On Apr 7, 2021, at 5:16 AM, riteshh <riteshh@xxxxxxxxxxxxx> wrote:
> > >>
> > >> On 21/04/07 03:01PM, Wen Yang wrote:
> > >>> From: Wen Yang <simon.wy@xxxxxxxxxxxxxxx>
> > >>>
> > >>> The kworker has occupied 100% of the CPU for several days:
> > >>> PID USER  PR  NI VIRT RES SHR S  %CPU  %MEM TIME+  COMMAND
> > >>> 68086 root 20 0  0    0   0   R  100.0 0.0  9718:18 kworker/u64:11
> > >>>
> > >>> And the stack obtained through sysrq is as follows:
> > >>> [20613144.850426] task: ffff8800b5e08000 task.stack: ffffc9001342c000
> > >>> [20613144.850438] Call Trace:
> > >>> [20613144.850439] [<ffffffffa0244209>]ext4_mb_new_blocks+0x429/0x550
> > [ext4]
> > >>> [20613144.850439]  [<ffffffffa02389ae>] ext4_ext_map_blocks+0xb5e/0xf30
> > [ext4]
> > >>> [20613144.850441]  [<ffffffffa0204b52>] ext4_map_blocks+0x172/0x620
> > [ext4]
> > >>> [20613144.850442]  [<ffffffffa0208675>] ext4_writepages+0x7e5/0xf00
> > [ext4]
> > >>> [20613144.850443]  [<ffffffff811c487e>] do_writepages+0x1e/0x30
> > >>> [20613144.850444]  [<ffffffff81280265>]
> > __writeback_single_inode+0x45/0x320
> > >>> [20613144.850444]  [<ffffffff81280ab2>] writeback_sb_inodes+0x272/0x600
> > >>> [20613144.850445]  [<ffffffff81280ed2>] __writeback_inodes_wb+0x92/0xc0
> > >>> [20613144.850445]  [<ffffffff81281238>] wb_writeback+0x268/0x300
> > >>> [20613144.850446]  [<ffffffff812819f4>] wb_workfn+0xb4/0x380
> > >>> [20613144.850447]  [<ffffffff810a5dc9>] process_one_work+0x189/0x420
> > >>> [20613144.850447]  [<ffffffff810a60ae>] worker_thread+0x4e/0x4b0
> > >>>
> > >>> The cpu resources of the cloud server are precious, and the server
> > >>> cannot be restarted after running for a long time, so a configuration
> > >>> parameter is added to prevent this endless loop.
> > >>
> > >> Strange, if there is a endless loop here. Then I would definitely see
> > >> if there is any accounting problem in pa->pa_count. Otherwise busy=1
> > >> should not be set everytime. ext4_mb_show_pa() function may help debug
> > this.
> > >>
> > >> If yes, then that means there always exists either a file preallocation
> > >> or a group preallocation. Maybe it is possible, in some use case.
> > >> Others may know of such use case, if any.
> >
> > > If this code is broken, then it doesn't make sense to me that we would
> > > leave it in the "run forever" state after the patch, and require a sysfs
> > > tunable to be set to have a properly working system?
> >
> > > Is there anything particularly strange about the workload/system that
> > > might cause this?  Filesystem is very full, memory is very low, etc?
> >
> > Hi Ritesh and Andreas,
> >
> > Thank you for your reply. Since there is still a faulty machine, we have
> > analyzed it again and found it is indeed a very special case:
> >
> >
> > crash> struct ext4_group_info ffff8813bb5f72d0
> > struct ext4_group_info {
> >   bb_state = 0,
> >   bb_free_root = {
> >     rb_node = 0x0
> >   },
> >   bb_first_free = 1681,
> >   bb_free = 0,
> 
> Not related to this issue, but above two variables values doesn't looks
> consistent.
> 
> >   bb_fragments = 0,
> >   bb_largest_free_order = -1,
> >   bb_prealloc_list = {
> >     next = 0xffff880268291d78,
> >     prev = 0xffff880268291d78     ---> *** The list is empty
> >   },
> 
> Ok. So when you collected the dump this list was empty.

No, it is not empty. It has a single element. Note that the structure is at
ffff8813bb5f72d0 so the pointers would have to be like ffff8813bb5f7xxx.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR