Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 2 Mar 2022 12:38:00 -0800 (PST)
From:   Hugh Dickins <hughd@google.com>
To:     Michal Hocko <mhocko@suse.com>
cc:     Andrew Morton <akpm@linux-foundation.org>, cgel.zte@gmail.com,
        naoya.horiguchi@nec.com, minchan@kernel.org, hannes@cmpxchg.org,
        rogerq@kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, guo.ziliang@zte.com.cn,
        Zeal Robot <zealci@zte.com.cn>,
        Ran Xiaokai <ran.xiaokai@zte.com.cn>,
        Jiang Xuexin <jiang.xuexin@zte.com.cn>,
        Yang Yang <yang.yang29@zte.com.cn>
Subject: Re: [PATCH linux-next] mm: swap: get rid of deadloop in swapin
 readahead
In-Reply-To: <Yh889CjjSxigdEAY@dhcp22.suse.cz>
Message-ID: <94de248f-94fa-91d-eb80-2ea75548282d@google.com>
References: <20220221111749.1928222-1-cgel.zte@gmail.com> <YhyAfSTkXFmIam9+@dhcp22.suse.cz> <20220228073315.c855e15b108089187a40d2b0@linux-foundation.org> <Yh889CjjSxigdEAY@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 2 Mar 2022, Michal Hocko wrote:
> 
> I might be really missing something but I really do not see how is this
> any different from the page allocator path which only does cond_resched
> as well (well, except for throttling but that might just not trigger).
> Or other paths which just do cond_resched while waiting for a progress
> somewhere else.
> 
> Not that I like this situation but !PREEMPT kernel with RT priority
> tasks is rather limited and full of potential priblems IMHO.

As I said in previous mail, I have really not given this as much
thought this time as I did in the 2018 mail thread linked there;
but have seen that it behaves more badly than I had imagined, in
any preemptive kernel - no need for RT.  We just don't have the
stats to show when this code here spins waiting on code elsewhere
that is sleeping.  I think the difference from most cond_resched()
places is that swapin is trying to collect together several factors
with minimal locking, and we should have added preempt_disable()s
when preemption was invented.  But it's only swap so we didn't notice.

Hugh