On Mon, Apr 13, 2026 at 08:22:44PM +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. > > 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. > > Does this make sense? > > Thanks, > Jinhui
it might be quite ok. what is missing is the analysis of whether we can actually get this error and what happens then.
