On Mon, Mar 1, 2021 at 7:57 AM Michal Hocko <mhocko@xxxxxxxx> wrote: > > On Mon 01-03-21 07:10:11, Shakeel Butt wrote: > > On Mon, Mar 1, 2021 at 4:12 AM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > > On Fri 26-02-21 16:00:30, Shakeel Butt wrote: > > > > On Fri, Feb 26, 2021 at 3:14 PM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote: > > > > > > > > > > Cc: Michal > > > > > > > > > > On 2/26/21 2:44 PM, Shakeel Butt wrote: > > > > > > On Fri, Feb 26, 2021 at 2:09 PM syzbot > > > > > > <syzbot+506c8a2a115201881d45@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > > <snip> > > > > > >> other info that might help us debug this: > > > > > >> > > > > > >> Possible interrupt unsafe locking scenario: > > > > > >> > > > > > >> CPU0 CPU1 > > > > > >> ---- ---- > > > > > >> lock(hugetlb_lock); > > > > > >> local_irq_disable(); > > > > > >> lock(slock-AF_INET); > > > > > >> lock(hugetlb_lock); > > > > > >> <Interrupt> > > > > > >> lock(slock-AF_INET); > > > > > >> > > > > > >> *** DEADLOCK *** > > > > > > > > > > > > This has been reproduced on 4.19 stable kernel as well [1] and there > > > > > > is a reproducer as well. > > > > > > > > > > > > It seems like sendmsg(MSG_ZEROCOPY) from a buffer backed by hugetlb. I > > > > > > wonder if we just need to make hugetlb_lock softirq-safe. > > > > > > > > > > > > [1] https://syzkaller.appspot.com/bug?extid=6383ce4b0b8ec575ad93 > > > > > > > > > > Thanks Shakeel, > > > > > > > > > > Commit c77c0a8ac4c5 ("mm/hugetlb: defer freeing of huge pages if in non-task > > > > > context") attempted to address this issue. It uses a work queue to > > > > > acquire hugetlb_lock if the caller is !in_task(). > > > > > > > > > > In another recent thread, there was the suggestion to change the > > > > > !in_task to in_atomic. > > > > > > > > > > I need to do some research on the subtle differences between in_task, > > > > > in_atomic, etc. TBH, I 'thought' !in_task would prevent the issue > > > > > reported here. But, that obviously is not the case. > > > > > > > > I think the freeing is happening in the process context in this report > > > > but it is creating the lock chain from softirq-safe slock to > > > > irq-unsafe hugetlb_lock. So, two solutions I can think of are: (1) > > > > always defer the freeing of hugetlb pages to a work queue or (2) make > > > > hugetlb_lock softirq-safe. > > > > > > There is __do_softirq so this should be in the soft IRQ context no? > > > Is this really reproducible with kernels which have c77c0a8ac4c5 > > > applied? > > > > Yes this is softirq context and syzbot has reproduced this on > > linux-next 20210224. > > Then how come this can ever be a problem? in_task() should exclude soft > irq context unless I am mistaken. > If I take the following example of syzbot's deadlock scenario then CPU1 is the one freeing the hugetlb pages. It is in the process context but has disabled softirqs (see __tcp_close()). CPU0 CPU1 ---- ---- lock(hugetlb_lock); local_irq_disable(); lock(slock-AF_INET); lock(hugetlb_lock); <Interrupt> lock(slock-AF_INET); So, this deadlock scenario is very much possible.
