Re: [PATCH rdma-next 2/2] RDMA/mana_ib: Implement get_dma_mr

[Jason Gunthorpe]

On Mon, Apr 22, 2024 at 09:12:46AM +0000, Konstantin Taranov wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > On Fri, Apr 19, 2024 at 09:14:14AM +0000, Konstantin Taranov wrote:
> > > > From: Jason Gunthorpe <jgg@xxxxxxxx> On Wed, Apr 17, 2024 at
> > > > 07:20:59AM -0700, Konstantin Taranov wrote:
> > > > > From: Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>
> > > > >
> > > > > Implement allocation of DMA-mapped memory regions.
> > > > >
> > > > > Signed-off-by: Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>
> > > > > ---
> > > > >  drivers/infiniband/hw/mana/device.c |  1 +
> > > > >  drivers/infiniband/hw/mana/mr.c     | 36
> > > > +++++++++++++++++++++++++++++
> > > > >  include/net/mana/gdma.h             |  5 ++++
> > > > >  3 files changed, 42 insertions(+)
> > > >
> > > > What is the point of doing this without supporting enough verbs to
> > > > allow a kernel ULP?
> > > >
> > >
> > > True, the proposed code is useless at this state.
> > > Nevertheless, mana_ib team aims to send kernel ULP patches after we
> > > are done with uverbs pathes (i.e., udata is not null). As this change
> > > does not conflict with the current effort, I decided to send this
> > > patch now. I can extend the series to make it more useful.
> > >
> > > Jason, could  you suggest a minimal list of ib_device_ops methods,
> > > that includes get_dma_mr, which can be approved?
> > 
> > Is there any chance you can send a single series to support a ULP. NVMe or
> > something like?
> 
> Sure, I can. I will investigate the way to make get_dma_mr used with fewer changes. 
> 
> Generally, I am wondering what would be easier for reviewers.
> Should I try to send short patch series enabling one feature, or should I actually try
> to produce long patch series that enable a use-case consisting of several features?

Generally the guideline is to avoid adding dead code, some exceptions
may be possible, but that should be the gold standard to try for,
IMHO.

If you want to support kernel ULPs then say witch kernel ULP you want
and send a series to make it work.

If that series is way too big then split it into two halfs and post
both at the start.

Jason