Re: [PATCH v3 1/2] drivers: usb/core/urb: Add URB_FREE_COHERENT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 24, 2022 at 10:43:34AM -0400, Alan Stern wrote:
> > 
> > > One way to solve this issue for usbip
> > > is to add some boilerplate transform
> > > from URB_* to USBIP_FLAGS_*
> > > as it is de facto uapi now.
> > 
> > It doesn't sound like a there is a compelling reason other than
> > "it would be nice to keep the flag values in numerical order".
> > 
> > I would not recommend this option. I am not seeing any value to adding
> > change URB_* to USBIP_FLAGS_* layer without some serious techinical
> > concerns.

The transfer_flag in usbip is de facto uapi,
That's why I'm proposing the USBIP_FLAGS_* way and
further more I think usbip could move some flags/structs
in usbip_common.h to include/uapi/linux/usb/usbip.h,
instead of the userspace copying them into their own
header.

I will start a new thread if Shuah think that is acceptable.

If this patch is to be landed, I think it should be
landed along with the usbip change so there would be no
userspace change;

Even without this patch, making usbip flags/structs uapi alone
is still worth doing.

> > 
> > > 
> > > Another way is to use 0x0400 for FREE_COHERENT.
> > > usbip will not take care of this bit as
> > > it would be masked.
> > > 
> > 
> > I would go with this option adding a clear comment with link to this
> > discussion.
> > 
> > > Cc Shuah Khan here since she is the maintainer
> > > on usbip.
> > > 
> > 
> > Thank you adding me to the discussion.
> 
> I can see this causing more problems in the future.  There's no hint in 
> include/linux/usb.h that any of the values it defines are part of a user 
> API.  If they are, they should be moved to include/uapi/linux/usb/.

I agree with this argument.

> 
> In general, if a user program depends on kernel details that are not 
> designed to be part of a user API, you should expect that the program 
> will sometimes break from one kernel version to another.
> 
> Yes, I know Linus insists that kernel changes should not cause 
> regressions in userspace, but the line has to be drawn somewhere.  
> Otherwise the kernel could never change at all.
> 
> Alan Stern



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux