On Wed, 2019-04-24 at 08:24 -0700, James Bottomley wrote: > On Wed, 2019-04-24 at 15:52 +0800, Ming Lei wrote: > > On Tue, Apr 23, 2019 at 08:37:15AM -0700, Bart Van Assche wrote: > > > On Tue, 2019-04-23 at 18:32 +0800, Ming Lei wrote: > > > > #define SCSI_INLINE_PROT_SG_CNT 1 > > > > > > > > +#define SCSI_INLINE_SG_CNT 2 > > > > > > So this patch inserts one kmalloc() and one kfree() call in the hot > > > path for every SCSI request with more than two elements in its > > > scatterlist? Isn't > > > > Slab or its variants are designed for fast path, and NVMe PCI uses > > slab for allocating sg list in fast path too. > > Actually, that's not really true base kmalloc can do all sorts of > things including kick off reclaim so it's not really something we like > using in the fast path. The only fast and safe kmalloc you can rely on > in the fast path is GFP_ATOMIC which will fail quickly if no memory > can easily be found. *However* the sg_table allocation functions are > all pool backed (lib/sg_pool.c), so they use the lightweight GFP_ATOMIC > mechanism for kmalloc initially coupled with a backing pool in case of > failure to ensure forward progress. > > So, I think you're both right: you shouldn't simply use kmalloc, but > this implementation doesn't, it uses the sg_table allocation functions > which correctly control kmalloc to be lightweight and efficient and > able to make forward progress.
Another concern is whether this change can cause a livelock. If the system is running out of memory and the page cache submits a write request with a scatterlist with more than two elements, if the kmalloc() for the scatterlist fails, will that prevent the page cache from making any progress with writeback? Bart.
