On Sat, Oct 3, 2026 at 6:39 PM Lorenzo Stoakes (ARM) <[email protected]> wrote: > > Move from the deprecated mmap hook to the new mmap_prepare hook. > > We are mapping kernel pages here, so use the discontiguous kernel mapping > mmap action to do so. > > Unwind the rather confusing loop and instead map as many pages as we can at > one time. > > Note that we do not need to pay attention to rsv_schp->k_use_sg here, as > the pages are populated for the length of the buffer at > rsv_schp->page_order granularity as compound pages. > > The discontiguous kernel page mapping logic handles the compound pages for > us. > > sfp->mmap_called keeps the buffer stable for us. As before it is never > cleared, so a failed mmap also leaves it set. > > We also remove some useless vma, vma->vm_file NULL checks - these will > always be non-NULL if you reached the mmap hook logic. > > We retain log output for consistency, but change what's output on page > mapping to indicate that sg_discontig_get() does the work now. > > Note that we drop the VMA_IO_BIT flag for the VMA here. It was never > necessary as we invoke alloc_pages() which gives us refcounted folios that > are fine for GUP to access (VMA_IO_BIT would prevent that). > > Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>
The code looks correct but I must disclaim that I have no experience with this driver. Reviewed-by: Suren Baghdasaryan <[email protected]> > --- > drivers/scsi/sg.c | 115 > ++++++++++++++++++++++++------------------------------ > 1 file changed, 51 insertions(+), 64 deletions(-) > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > index 5408f002e6c0..3f9e08725602 100644 > --- a/drivers/scsi/sg.c > +++ b/drivers/scsi/sg.c > @@ -1212,85 +1212,72 @@ sg_fasync(int fd, struct file *filp, int mode) > return fasync_helper(fd, filp, mode, &sfp->async_qp); > } > > -static vm_fault_t > -sg_vma_fault(struct vm_fault *vmf) > +static int sg_discontig_init(void *vm_private_data, void **private) > { > - struct vm_area_struct *vma = vmf->vma; > - Sg_fd *sfp; > - unsigned long offset, len, sa; > - Sg_scatter_hold *rsv_schp; > - int k, length; > - > - if ((NULL == vma) || (!(sfp = (Sg_fd *) vma->vm_private_data))) > - return VM_FAULT_SIGBUS; > - rsv_schp = &sfp->reserve; > - offset = vmf->pgoff << PAGE_SHIFT; > - if (offset >= rsv_schp->bufflen) > - return VM_FAULT_SIGBUS; > - SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sfp->parentdp, > - "sg_vma_fault: offset=%lu, scatg=%d\n", > - offset, rsv_schp->k_use_sg)); > - sa = vma->vm_start; > - length = 1 << (PAGE_SHIFT + rsv_schp->page_order); > - for (k = 0; k < rsv_schp->k_use_sg && sa < vma->vm_end; k++) { > - len = vma->vm_end - sa; > - len = (len < length) ? len : length; > - if (offset < len) { > - struct page *page = rsv_schp->pages[k] + (offset >> > PAGE_SHIFT); > - get_page(page); /* increment page count */ > - vmf->page = page; > - return 0; /* success */ > - } > - sa += len; > - offset -= len; > + const unsigned long req_sz = (unsigned long)*private; > + Sg_fd *sfp = vm_private_data; > + Sg_scatter_hold *rsv_schp = &sfp->reserve; > + int err = 0; > + > + mutex_lock(&sfp->f_mutex); > + if (req_sz > rsv_schp->bufflen) { > + err = -ENOMEM; /* cannot map more than reserved buffer */ > + goto out; > + } > + sfp->mmap_called = 1; /* Prevents changes to buffer size. */ > +out: > + mutex_unlock(&sfp->f_mutex); > + return err; > +} > + > +static int > +sg_discontig_get(struct discontig_kernel_page_state *state) > +{ > + Sg_fd *sfp = state->vm_private_data; > + Sg_scatter_hold *rsv_schp = &sfp->reserve; > + const unsigned int order = rsv_schp->page_order; > + const pgoff_t nr_pages = state->nr_pages_mapped; > + > + if (nr_pages >= (rsv_schp->bufflen >> PAGE_SHIFT)) { > + discontig_kernel_map_abort(state); > + return 0; > } > > - return VM_FAULT_SIGBUS; > + SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sfp->parentdp, > + "%s: offset=%lu, scatg=%d\n", __func__, > + nr_pages << PAGE_SHIFT, > rsv_schp->k_use_sg)); > + > + discontig_kernel_map_page(state, rsv_schp->pages[nr_pages >> order]); > + return 0; > } > > -static const struct vm_operations_struct sg_mmap_vm_ops = { > - .fault = sg_vma_fault, > +static const struct discontig_kernel_page_ops sg_discontig_ops = { > + .init = sg_discontig_init, > + .get = sg_discontig_get, > }; > > static int > -sg_mmap(struct file *filp, struct vm_area_struct *vma) > +sg_mmap_prepare(struct vm_area_desc *desc) > { > - Sg_fd *sfp; > - unsigned long req_sz, len, sa; > - Sg_scatter_hold *rsv_schp; > - int k, length; > - int ret = 0; > + Sg_fd *sfp = desc->file->private_data; > + const unsigned long req_sz = vma_desc_size(desc); > > - if ((!filp) || (!vma) || (!(sfp = (Sg_fd *) filp->private_data))) > + if (!sfp) > return -ENXIO; > - req_sz = vma->vm_end - vma->vm_start; > + > SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sfp->parentdp, > "sg_mmap starting, vm_start=%p, > len=%d\n", > - (void *) vma->vm_start, (int) req_sz)); > - if (vma->vm_pgoff) > + (void *) desc->start, (int) req_sz)); > + > + if (desc->pgoff) > return -EINVAL; /* want no offset */ > - rsv_schp = &sfp->reserve; > - mutex_lock(&sfp->f_mutex); > - if (req_sz > rsv_schp->bufflen) { > - ret = -ENOMEM; /* cannot map more than reserved buffer */ > - goto out; > - } > > - sa = vma->vm_start; > - length = 1 << (PAGE_SHIFT + rsv_schp->page_order); > - for (k = 0; k < rsv_schp->k_use_sg && sa < vma->vm_end; k++) { > - len = vma->vm_end - sa; > - len = (len < length) ? len : length; > - sa += len; > - } > + vma_desc_set_flags(desc, VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT); > + desc->private_data = sfp; > > - sfp->mmap_called = 1; > - vm_flags_set(vma, VM_IO | VM_DONTEXPAND | VM_DONTDUMP); > - vma->vm_private_data = sfp; > - vma->vm_ops = &sg_mmap_vm_ops; > -out: > - mutex_unlock(&sfp->f_mutex); > - return ret; > + mmap_action_map_discontig_kernel_pages(desc, (void *)req_sz, > + &sg_discontig_ops); > + return 0; > } > > static void > @@ -1415,7 +1402,7 @@ static const struct file_operations sg_fops = { > .unlocked_ioctl = sg_ioctl, > .compat_ioctl = compat_ptr_ioctl, > .open = sg_open, > - .mmap = sg_mmap, > + .mmap_prepare = sg_mmap_prepare, > .release = sg_release, > .fasync = sg_fasync, > }; > > -- > 2.55.0 >
