Re: [PATCH RFC 0/2] mempool based chained scatterlist alloc/free api api

[Christoph Hellwig]

On Tue, Mar 15, 2016 at 04:12:03PM -0700, James Bottomley wrote:
> motion.  If you add in one patch and remove in another the code motion
> trackers don't see it.

Agreed, having the move in a single patch would be nice.

> Thirdly, are you sure the pool structure for NVMe should be the same as
> for SCSI?  We don't do buddy pools for 1,2 or 4 entry transactions in
> SCSI just basically because of heuristics, but the packetised io
> characteristics of NVMe make single entry lists more likely for it,
> don't they?

Not really.  NVMe doesn't really do packetized I/O.  And while people
were setting all kinds of nomerge flags early on we're getting rid
of them and are seeing similar I/O patterns to fast SCSI devices
now.

NVMe over PCIe still uses the crazy PRPs by default, which aren't
very suitable for this allocator (someone will have to come up
with a good mempool for it eventually, though), but we're developing
a set of new drivers transporting NVMe command which use SGLs very
similar to most SCSI controllers, so using the same SGL allocator
is a very natural choice.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/2] mempool based chained scatterlist alloc/free api api

[Ming Lin]

On Tue, 2016-03-15 at 16:12 -0700, James Bottomley wrote:
> On Tue, 2016-03-15 at 15:39 -0700, Ming Lin wrote:
> > From: Ming Lin 
> > 
> > Hi list,
> > 
> > This moves the mempool based chained scatterlist alloc/free code
> > from
> > scsi_lib.c to lib/scatterlist.c.
> > 
> > So other drivers(for example, the under development NVMe over
> > fabric 
> > drivers) can also use it.
> > 
> > Ming Lin (2):
> >   scatterlist: add mempool based chained SG alloc/free api
> >   scsi: use the new chained SG api
> > 
> >  drivers/scsi/scsi_lib.c | 129 ++
> > --
> >  include/linux/scatterlist.h |  12 
> >  lib/scatterlist.c   | 156
> > 
> >  3 files changed, 175 insertions(+), 122 deletions(-)
> 
> I'd really rather this were a single patch so git can tell us the
> code
> motion.  If you add in one patch and remove in another the code
> motion
> trackers don't see it.
> 
> Secondly, you said "This copied code from scsi_lib.c to scatterlist.c
> and modified it a bit" could you move in one patch and modify in
> another, so we can see exactly what you're changing.

The modification is mostly about structure names and function names
changes.

I can do it in a single patch.

> 
> Thirdly, are you sure the pool structure for NVMe should be the same
> as
> for SCSI?  We don't do buddy pools for 1,2 or 4 entry transactions in
> SCSI just basically because of heuristics, but the packetised io
> characteristics of NVMe make single entry lists more likely for it,
> don't they?

Not sure about this, but the nvme-pci driver may not use this api,
because it also has a PRP lists except for the SG lists.

But nvme-over-rdma/nvme-over-fiber-channel driver is good to use this
api.

> 
> James
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/2] mempool based chained scatterlist alloc/free api api

[James Bottomley]

On Tue, 2016-03-15 at 15:39 -0700, Ming Lin wrote:
> From: Ming Lin 
> 
> Hi list,
> 
> This moves the mempool based chained scatterlist alloc/free code from
> scsi_lib.c to lib/scatterlist.c.
> 
> So other drivers(for example, the under development NVMe over fabric 
> drivers) can also use it.
> 
> Ming Lin (2):
>   scatterlist: add mempool based chained SG alloc/free api
>   scsi: use the new chained SG api
> 
>  drivers/scsi/scsi_lib.c | 129 ++--
>  include/linux/scatterlist.h |  12 
>  lib/scatterlist.c   | 156 
> 
>  3 files changed, 175 insertions(+), 122 deletions(-)

I'd really rather this were a single patch so git can tell us the code
motion.  If you add in one patch and remove in another the code motion
trackers don't see it.

Secondly, you said "This copied code from scsi_lib.c to scatterlist.c
and modified it a bit" could you move in one patch and modify in
another, so we can see exactly what you're changing.

Thirdly, are you sure the pool structure for NVMe should be the same as
for SCSI?  We don't do buddy pools for 1,2 or 4 entry transactions in
SCSI just basically because of heuristics, but the packetised io
characteristics of NVMe make single entry lists more likely for it,
don't they?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 0/2] mempool based chained scatterlist alloc/free api api

[Ming Lin]

From: Ming Lin 

Hi list,

This moves the mempool based chained scatterlist alloc/free code from
scsi_lib.c to lib/scatterlist.c.

So other drivers(for example, the under development NVMe over fabric drivers)
can also use it.

Ming Lin (2):
  scatterlist: add mempool based chained SG alloc/free api
  scsi: use the new chained SG api

 drivers/scsi/scsi_lib.c | 129 ++--
 include/linux/scatterlist.h |  12 
 lib/scatterlist.c   | 156 
 3 files changed, 175 insertions(+), 122 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html