linux-kernel - Re: [PATCH v3 1/3] usb: core: add dma-noncoherent buffer alloc and free API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5wzejydo3aaqoylwcwscjg2m4sovprcwtywkdahshb2f6uigez@zwxk522nnbrj>
Date: Thu, 3 Jul 2025 16:40:50 +0800
From: Xu Yang <xu.yang_2@....com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: ezequiel@...guardiasur.com.ar, mchehab@...nel.org, 
	laurent.pinchart@...asonboard.com, hdegoede@...hat.com, gregkh@...uxfoundation.org, 
	mingo@...nel.org, tglx@...utronix.de, viro@...iv.linux.org.uk, 
	thomas.weissschuh@...utronix.de, dafna.hirschfeld@...labora.com, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org, imx@...ts.linux.dev, jun.li@....com
Subject: Re: [PATCH v3 1/3] usb: core: add dma-noncoherent buffer alloc and
 free API

On Wed, Jul 02, 2025 at 03:20:33PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 02, 2025 at 07:02:20PM +0800, Xu Yang wrote:
> > This will add usb_alloc_noncoherent() and usb_free_noncoherent()
> > functions to support alloc and free buffer in a dma-noncoherent way.
> > 
> > To explicit manage the memory ownership for the kernel and device,
> > this will also add usb_dma_noncoherent_sync_for_cpu/device() functions
> > and call it at proper time.  The management requires the user save
> > sg_table returned by usb_alloc_noncoherent() to urb->sgt.
> 
> ...
> 
> > +	if ((urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP) &&
> > +	    urb->sgt) {
> 
> Something weird with the indentation. Here you wrapped, but the very next line
> is not, and they both satisfy 80 limit (even though we have it relaxed now).

Indeed. Okay, I'll not wrap this line.

> 
> > +		dma_sync_sgtable_for_cpu(hcd->self.sysdev, urb->sgt, dir);
> > +		if (dir == DMA_FROM_DEVICE)
> > +			invalidate_kernel_vmap_range(urb->transfer_buffer,
> > +						     urb->transfer_buffer_length);
> > +	}
> 
> ...
> 
> > +
> > +		return ret;
> >  	}
> > +
> > +	if ((urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP) &&
> > +	    urb->sgt) {
> 
> Why not
> 
> 	} else if {
> 
> and drop the above return ret; ?

Okay.

Thanks,
Xu Yang

> 
> > +		if (dir == DMA_TO_DEVICE)
> > +			flush_kernel_vmap_range(urb->transfer_buffer,
> > +						urb->transfer_buffer_length);
> > +		dma_sync_sgtable_for_device(hcd->self.sysdev, urb->sgt, dir);
> > +	}
> > +
> >  	return ret;
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>