From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D2D583CCFCE for ; Mon, 13 Apr 2026 13:33:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776087186; cv=none; b=VnaCJLjZ8ZP/7+DrBPjYW8n/ZzMMdbPM90hm3nSqNo8SHwcDAX1apFjaP5ys7wu/inGEia/QsiGtIaupObpCiUTh+qhIM6jxwHZFNpJ/xgDwkNBbs4JMBgxaEdd6ly/+73H4QbyoEXd9U9rApOHsT9R5rtHdV7ah7OLJmBwwCC0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776087186; c=relaxed/simple; bh=cNzldwXv7EEzDpZRh1GYW+jmgvH87VS+HrKz6Gcd/no=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=jFC/PQ9ez2udijiBW0rXuioUMlflD2MzKT0GwDKZlWVzpSgG7B6G/dilKpipo1l2PA/p5X9+DULW87ncbyXw6+6b8kxOupCwJUxiSn95odYg3cwy1bEFr3AKxP+5Fs5yAcAB4KlrtoN/dABMk06A2Hlj8q+fg/7U4KpM2go3+Qw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=tMjqkzVD; arc=none smtp.client-ip=209.85.128.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="tMjqkzVD" Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-488aa77a06eso75221255e9.0 for ; Mon, 13 Apr 2026 06:33:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1776087183; x=1776691983; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=NAr+JGO4ktVEFi2tj7VFYFMXV+IisACoZSycsDEBlws=; b=tMjqkzVDXVZtpAI2IyByfhpXzYWKB/56awuVFz6yaze3Lzwyro9XLAb2eKpZAWMAdi mvVE8rcNUZbuzkfS5TN9WKnEPn+yIhkoSY5dtgIE0MwiCYqCo3Wl320Tu0EH4EXwtn4V 3Jtlo9K1sa/5RYX/yIbzAauJX24YFEI8xp7wDsyYtLFmG+CmMz6Ov1UYSw93UEFqvRdR XsYm7+HT8ecIK+2mJEOFcn0K/Pqa4/0l1p/CGkf1DSFN0rASSt2HZ+afNpfDPbSSMGQJ PEtWdgJTozBBeTwnfAG9SBOIo4HAiAJp3QTiWxLu0Hr0/DGSEcoxheL3nNALkKpZwZjF o9Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776087183; x=1776691983; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=NAr+JGO4ktVEFi2tj7VFYFMXV+IisACoZSycsDEBlws=; b=SNOqf1FvV80jxVGrIBNM13cendxQ5nOSBb+hHQFc3n7LaOaiBt1nQXC+AUYsha62BY HIR2LFWjtfGs2dIVV3neplk2CkpJsK6FwahAb/X5s9rh5yAoFM7XCXw6TC5NBaHL66Vm 4r7wzY5z4uBaLNwFpsg9jo8RFx2bVDZx7pfOSOyjX9LhdVIpq9Z0vyY+L1r11Rbg/iTb AL7xDptvsbR62yGokO6iOuOo6Sx3Wu55qy8KAFwpxGofw4bQXROeoe7BKp9KhcUFmR0r z2wpWS2PUJ6olXWXjZNdatE0eDqw0djNBrmx2jQui56C3JBoyyiuXMr4McBiU6h6bhFn IkJA== X-Forwarded-Encrypted: i=1; AFNElJ8IkUHTRs0LOh7yL80Shs+fzfDweQHaO6dGcprxJqZvVvxfmLiDMlNeRWDWzZ4dysDM43Rg3Rnr6G6zfP0=@vger.kernel.org X-Gm-Message-State: AOJu0YzWJxU2AFOvGqMc/REvOvWPAVmHCdV/kuT1U6f3OGqfN/HzwJI1 DPB4ooiaR3+DMsC5WpeVt6LTUub+3gK3AVJTO4O2T9+lkq7oyY+7OfYk X-Gm-Gg: AeBDiesVQfQIrJrtxKhvwHJMuXsLR5zP1UBBwfuIZaswaBnSjud2v/sSjFx2VlKM9jX DnDNmcqx5tq7NVI1ipSi6Ob+h6usqUmCp1unFAW08bP7CO/OIM3iULKzm9rVyoK+DBiHZgAVY4a CfP88zfiKhiYhSPBwJGe76Hh0+DqyZ8+TpWp1e/opIR+XZHcNfo3FIb+RdPPhSYQpDjnhKhn7GG eiGR2y4hFiNU/d3qCaPou3HwOWblpnersxwjtlff+Naw7VJKhPY9Nc2Obm/TUxQyRzsrvzFGAUS Lh1RZG1wderHWZbpf+an4vmjTwrcOGEE1isSdDRk2+6wPC/UWi8MgXzDrbabyzH8K82WEtYzvKD mSObSEFflbWjRdajmPYQHNA/nKQ9NRCb8a6A+Tmcl0q3ff0FEeTWFdkBP1OViDojVYBKFrQb4Po 2zCyCMDGLmxL4FeCF86tIN8D5h2VWNuPl/eG0PqQyGcFM9vZncaDpkomjM9AAzxeIJ X-Received: by 2002:a05:600c:4443:b0:485:3a03:ceca with SMTP id 5b1f17b1804b1-488d686c044mr182576135e9.23.1776087183006; Mon, 13 Apr 2026 06:33:03 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43d762decf6sm18410380f8f.8.2026.04.13.06.33.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Apr 2026 06:33:02 -0700 (PDT) Date: Mon, 13 Apr 2026 14:33:00 +0100 From: David Laight To: "Jinhui Guo" Cc: "Michael S. Tsirkin" , Eugenio =?UTF-8?B?UMOpcmV6?= , "Jason Wang" , "Jiri Pirko" , "Xuan Zhuo" , , , Subject: Re: [PATCH] virtio_pci_modern: Use GFP_ATOMIC with spin_lock_irqsave held in virtqueue_exec_admin_cmd() Message-ID: <20260413143300.16922e4f@pumpkin> In-Reply-To: <20260413122244.534-1-guojinhui.liam@bytedance.com> References: <20260413101759.6323fb68@pumpkin> <20260413122244.534-1-guojinhui.liam@bytedance.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Mon, 13 Apr 2026 20:22:44 +0800 "Jinhui Guo" wrote: > On Mon, Apr 13, 2026 at 10:17:59 +0100, David Laight wrote: > > Or do the allocate before acquiring the lock (and free it not used > > in the error path). > > Hi David, > > Thanks for the suggestion. > > Pre-allocating the memory outside the lock is indeed a good practice, > but unfortunately it doesn't work in this specific virtqueue context. > > The kmalloc() in question is not happening at the virtqueue_exec_admin_cmd() > level. Instead, it is deeply embedded inside virtqueue_add_sgs() > (specifically, in functions like alloc_indirect_split() or > virtqueue_add_indirect_packed()) to allocate indirect descriptors when > multiple SG elements are provided. > > As a caller, we have no mechanism to pre-allocate this indirect descriptor > memory and pass it down to virtqueue_add_sgs(). Furthermore, virtqueue_add_sgs() > needs to atomically check the queue's num_free status, allocate the indirect > table if necessary, and update the queue pointers. All these operations > must be protected by admin_vq->lock to prevent concurrent admin command > submissions from corrupting the virtqueue state. It just sounds non-trivial... > > Therefore, allocating before acquiring the lock isn't feasible here, and > replacing GFP_KERNEL with GFP_ATOMIC (with a proper sleepable retry upon > failure) seems to be the more viable fix. The sleep-retry isn't really ideal - and may not make progress. An 'interesting' solution would be to return the size of the kmalloc() that failed, kmalloc() and kfree() a buffer of that size and hope it is still available for the retry. For a quick read of the code it is always a constant multiplied by the number of fragments. Although I only found kmalloc() in the 'indirect' paths. I didn't spot what happens if the ring itself is full. David > > Does this make sense? > > Thanks, > Jinhui