Re: [PATCH v4 09/14] nvme-pci: add SGL support for the dmabuf path

[Pavel Begunkov]

On 7/29/26 12:31, Christoph Hellwig wrote:
> On Wed, Jul 29, 2026 at 03:47:23PM +0530, Anuj Gupta/Anuj Gupta wrote:
> > > But I also don't understand what the use case for this function
> > > is to start with.  struct sg_table tells us how many segments
> > > exist on the DMA side in the nents member, which should be just
> > > fine for the SGL threshold calculation.
> >
> > sg_table->nents covers the entire exported buffer (<=1GiB), while a
> > request only covers a subrange[bi_offset, bi_offset+payload). Using
> > nents would overcount the request's segments.
>
> Urgg, yes.
>
> > > > +	if (!entries)
> > > > +		return BLK_STS_IOERR;
> > > > +	if (entries > NVME_MAX_SEGS)
> > > > +		return BLK_STS_AGAIN;
> > >
> > > Given that the block layer enforced data in rw/command and the
> > > max_segments limit, why do we need the extra check here?
> >
> > A dmabuf bio reports nsegs=1 (bio_split_io_at) to the block layer, so
> > max_segments isn't enforced against the SG entries actually spanned by
> > the request. Hence the explicit check.
>
> We'll need to expose the actual nsegs to the block layer and split
> based on that.  Otherwise I/O might work or fail based on the device
> capabilities.

It was exposed in early version as I was passing a [{dma,len}, ...]
array, but we moved from that. Maybe I should put the minimum
segment size in the map structure, and (possibly over) split using
that for now? Keith had this chunk in his patches:

+		int offset = offset_in_page(bio->bi_iter.bi_bvec_done);
+
+		nsegs = ALIGN(bio->bi_iter.bi_size + offset, PAGE_SIZE) >>
+								PAGE_SHIFT;
+		if (bio->bi_iter.bi_size > max_bytes) {
+			bytes = max_bytes;
+			nsegs = (bytes + offset) >> PAGE_SHIFT;
+		} else if (nsegs > lim->max_segments) {
+			nsegs = lim->max_segments;
+			bytes = PAGE_SIZE * nsegs - offset;
+		} else {
+			*segs = nsegs;
+			return NULL;
+		}

--
Pavel Begunkov