From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net V3 1/2] ptr_ring: try vmalloc() when kmalloc() fails Date: Fri, 9 Feb 2018 11:49:12 +0800 Message-ID: References: <1518062365-8596-1-git-send-email-jasowang@redhat.com> <20180208063509-mutt-send-email-mst@kernel.org> <2e3978d8-8bf1-2f0c-b446-a5dd65c7ac94@redhat.com> <20180208211636-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: netdev@vger.kernel.org To: "Michael S. Tsirkin" Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:59994 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752207AbeBIDtQ (ORCPT ); Thu, 8 Feb 2018 22:49:16 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 463EB8182D17 for ; Fri, 9 Feb 2018 03:49:16 +0000 (UTC) In-Reply-To: <20180208211636-mutt-send-email-mst@kernel.org> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018年02月09日 03:17, Michael S. Tsirkin wrote: > On Thu, Feb 08, 2018 at 02:58:40PM +0800, Jason Wang wrote: >> On 2018年02月08日 12:45, Michael S. Tsirkin wrote: >>> On Thu, Feb 08, 2018 at 11:59:24AM +0800, Jason Wang wrote: >>>> This patch switch to use kvmalloc_array() for using a vmalloc() >>>> fallback to help in case kmalloc() fails. >>>> >>>> Reported-by:syzbot+e4d4f9ddd4295539735d@syzkaller.appspotmail.com >>>> Fixes: 2e0ab8ca83c12 ("ptr_ring: array based FIFO for pointers") >>> I guess the actual patch is the one that switches tun to ptr_ring. >> I think not, since the issue was large allocation. >> >>> In fact, I think the actual bugfix is patch 2/2. This specific one >>> just makes kmalloc less likely to fail but that's >>> not what syzbot reported. >> Agree. >> >>> Then I would add this patch on top to make kmalloc less likely to fail. >> Ok. >> >>>> Signed-off-by: Jason Wang >>>> --- >>>> include/linux/ptr_ring.h | 10 +++++----- >>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h >>>> index 1883d61..2af71a7 100644 >>>> --- a/include/linux/ptr_ring.h >>>> +++ b/include/linux/ptr_ring.h >>>> @@ -466,7 +466,7 @@ static inline int ptr_ring_consume_batched_bh(struct ptr_ring *r, >>>> static inline void **__ptr_ring_init_queue_alloc(unsigned int size, gfp_t gfp) >>>> { >>>> - return kcalloc(size, sizeof(void *), gfp); >>>> + return kvmalloc_array(size, sizeof(void *), gfp | __GFP_ZERO); >>>> } >>>> static inline void __ptr_ring_set_size(struct ptr_ring *r, int size) >>> This implies a bunch of limitations on the flags. From kvmalloc_node >>> docs: >>> >>> * Reclaim modifiers - __GFP_NORETRY and __GFP_NOFAIL are not supported. >>> * __GFP_RETRY_MAYFAIL is supported, and it should be used only if kmalloc is >>> * preferable to the vmalloc fallback, due to visible performance drawbacks. >>> >>> Fine with all the current users, but if we go this way, please add >>> documentation so future users don't misuse this API. >> I suspect this is somehow a overkill since this means we need sync with >> mm/vmalloc changes in the future to keep it synced. >> >>> Alternatively, test flags and call kvmalloc or kcalloc? >> Similar to the above issue, I would rather leave it as is. >> >> Thanks > How do we prevent someone from inevitably trying to use this with > GFP_ATOMIC? > Well, we somehow can't prevent this even if there's a documentation, that's why there's a BUG() in vmalloc code I think. And kvmalloc also requires GFP_KERNEL otherewise another WARN(). So looks like the WARN()/BUG() should be sufficient? Thanks Another thing is kvm