Thread (61 messages) 61 messages, 2 authors, 2021-01-15

Re: [PATCH v13 43/45] sg: no_dxfer: move to/from kernel buffers

From: Hannes Reinecke <hare@suse.de>
Date: 2021-01-15 16:18:13

On 1/14/21 6:11 PM, Douglas Gilbert wrote:
On 2021-01-14 2:30 a.m., Hannes Reinecke wrote:
quoted
On 1/13/21 11:45 PM, Douglas Gilbert wrote:
quoted
When the NO_DXFER flag is use on a command/request, the data-in
and data-out buffers (if present) should not be ignored. Add
sg_rq_map_kern() function to handle this. Uses a single bio with
multiple bvec_s usually each holding multiple pages, if necessary.
The driver default element size is 32 KiB so if PAGE_SIZE is 4096
then get_order()==3 .

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
  drivers/scsi/sg.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 59 insertions(+)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index a00f488ee5e2..ad97f0756a9c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2865,6 +2865,63 @@ exit_sg(void)
      idr_destroy(&sg_index_idr);
  }
+static struct bio *
+sg_mk_kern_bio(int bvec_cnt)
+{
+    struct bio *biop;
+
+    if (bvec_cnt > BIO_MAX_PAGES)
+        return NULL;
+    biop = bio_alloc(GFP_ATOMIC, bvec_cnt);
+    if (!biop)
+        return NULL;
+    biop->bi_end_io = bio_put;
Huh? That is the default action, is it not?
So why specify it separately?
I'll check. That code snippet is copied from NVMe which has equivalent
code: moving storage data to/from _kernel_ buffers. Later in the driver
rewrite, bios are re-used, so if any earlier path puts a different
value in biop->bi_end_io, I'm screwed without that line. So I assumed
the NVMe code did it for a good reason.
Re-used? Uh-oh.
But okay, then it kinda makes sense.

[ .. ]
quoted
Why do you need to do the additional mapping?
I don't understand this question.
quoted
And doens't the 'NO_DXFER' flag indicate that _no_ data should be 
transferred?
NO, it absolutely does not mean that! With indirect IO (i.e. the 
default) there
are two transfers, taking a READ operation is an example:
    1) transfer user data from the device (a LU) to the internal buffer 
allocated
       by the sg driver. LLD arranges that transfer.
    2) transfer that user data from the internal buffer to the user 
space as
       indicated by the call to ioctl(SG_IO) or its alternatives. This 
transfer
       is driven by the sg driver using copy_to_user().

The SG_FLAG_NO_DXFER flag skips step 2) _not_ 1) .

The SG_FLAG_NO_DXFER flag has been in the sg driver since 1998. Sometime 
between
2010 and now that functionality was quietly dropped. Tony Battersby for one
seemed quite peeved when I told him that functionality had been silently
dropped.
Ah. Now that makes sense. Data transfer with not data transfer.
You should've said :-)

I'll have another look with that in mind.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help