| Message ID | 20190301172238.GA23800@kroah.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 8e6b85945155da5af95dc0fa58ae38b86428deea |
| Headers | show |
| Series | USB: usb.h: tweak struct urb to remove wasted space | expand |
On Fr, 2019-03-01 at 18:22 +0100, Greg Kroah-Hartman wrote: > By moving one field around in 'struct urb' we reduce the size of the > structure by 8 bytes. If you are going for this I have to ask why unlink and status are full size ints anyway. Regards Oliver
On Fri, Mar 01, 2019 at 08:44:13PM +0100, Oliver Neukum wrote: > On Fr, 2019-03-01 at 18:22 +0100, Greg Kroah-Hartman wrote: > > By moving one field around in 'struct urb' we reduce the size of the > > structure by 8 bytes. > > If you are going for this I have to ask why unlink and status > are full size ints anyway. History, they hold a "normal" negative error code. I guess we could turn them into s16, if it really matters, but that feels odd to me. thanks, greg k-h
On Sa, 2019-03-02 at 09:00 +0100, Greg Kroah-Hartman wrote: > On Fri, Mar 01, 2019 at 08:44:13PM +0100, Oliver Neukum wrote: > > On Fr, 2019-03-01 at 18:22 +0100, Greg Kroah-Hartman wrote: > > > By moving one field around in 'struct urb' we reduce the size of the > > > structure by 8 bytes. > > > > If you are going for this I have to ask why unlink and status > > are full size ints anyway. > > History, they hold a "normal" negative error code. I guess we could > turn them into s16, if it really matters, but that feels odd to me. Returning internal error codes to user space is a bug. Regards Oliver
On Sat, Mar 02, 2019 at 02:51:41PM +0100, Oliver Neukum wrote: > On Sa, 2019-03-02 at 09:00 +0100, Greg Kroah-Hartman wrote: > > On Fri, Mar 01, 2019 at 08:44:13PM +0100, Oliver Neukum wrote: > > > On Fr, 2019-03-01 at 18:22 +0100, Greg Kroah-Hartman wrote: > > > > By moving one field around in 'struct urb' we reduce the size of the > > > > structure by 8 bytes. > > > > > > If you are going for this I have to ask why unlink and status > > > are full size ints anyway. > > > > History, they hold a "normal" negative error code. I guess we could > > turn them into s16, if it really matters, but that feels odd to me. > > Returning internal error codes to user space is a bug. Where does status and unlink end up going to userspace? Maybe through usbfs? confused, greg k-h
On Sa, 2019-03-02 at 15:36 +0100, Greg Kroah-Hartman wrote: > On Sat, Mar 02, 2019 at 02:51:41PM +0100, Oliver Neukum wrote: > > On Sa, 2019-03-02 at 09:00 +0100, Greg Kroah-Hartman wrote: > > > On Fri, Mar 01, 2019 at 08:44:13PM +0100, Oliver Neukum wrote: > > > > On Fr, 2019-03-01 at 18:22 +0100, Greg Kroah-Hartman wrote: > > > > > By moving one field around in 'struct urb' we reduce the size of the > > > > > structure by 8 bytes. > > > > > > > > If you are going for this I have to ask why unlink and status > > > > are full size ints anyway. > > > > > > History, they hold a "normal" negative error code. I guess we could > > > turn them into s16, if it really matters, but that feels odd to me. > > > > Returning internal error codes to user space is a bug. > > Where does status and unlink end up going to userspace? Maybe through > usbfs? Nowhere, I hope. Sorry for the confusion. I hope I fixed that some time ago. My point here is that using the external errnos for internal error report is not an advantage. In fact, it is a bad idea. A byte would be ample to record internal errors. Regards Oliver
diff --git a/include/linux/usb.h b/include/linux/usb.h index 5e49e82c4368..4229eb74bd2c 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1545,10 +1545,10 @@ typedef void (*usb_complete_t)(struct urb *); struct urb { /* private: usb core and host controller only fields in the urb */ struct kref kref; /* reference count of the URB */ + int unlinked; /* unlink error code */ void *hcpriv; /* private data for host controller */ atomic_t use_count; /* concurrent submissions counter */ atomic_t reject; /* submissions will fail */ - int unlinked; /* unlink error code */ /* public: documented fields in the urb that can be used by drivers */ struct list_head urb_list; /* list head for use by the urb's
By moving one field around in 'struct urb' we reduce the size of the structure by 8 bytes. Before the patch on x86_64 the overall size of the structure as reported by pahole was: /* size: 192, cachelines: 3, members: 30 */ /* sum members: 184, holes: 2, sum holes: 8 */ After the patch we now have: /* size: 184, cachelines: 3, members: 30 */ /* last cacheline: 56 bytes */ Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- include/linux/usb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)