On Wed, 2019-04-24 at 08:32 -0700, Bart Van Assche wrote: > 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? It's pool backed, as I said. Is the concern there isn't enough depth in the pools for a large write? James
