| Message ID | 20220812225633.3287847-1-kbusch@fb.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fs: don't randomized kiocb fields | expand |
On 8/12/22 4:56 PM, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > This is a size sensitive structure and randomizing can introduce extra > padding that breaks io_uring's fixed size expectations. There are few > fields here as it is, half of which need a fixed order to optimally > pack, so the randomization isn't providing much. I'll add the link to the previous discussion as well in the commit: Link: https://lore.kernel.org/io-uring/b6f508ca-b1b2-5f40-7998-e4cff1cf7212@kernel.dk/ as it's pretty useful for context.
s/randomized/randomize/ in the subject?
On 8/13/22 1:59 AM, Christoph Hellwig wrote: > s/randomized/randomize/ > > in the subject? I did notice that one and made the edit yesterday.
On Fri, Aug 12, 2022 at 03:56:33PM -0700, Keith Busch wrote: > struct kiocb { > struct file *ki_filp; > - > - /* The 'ki_filp' pointer is shared in a union for aio */ > - randomized_struct_fields_start > - > loff_t ki_pos; > void (*ki_complete)(struct kiocb *iocb, long ret); > void *private; > int ki_flags; > u16 ki_ioprio; /* See linux/ioprio.h */ > struct wait_page_queue *ki_waitq; /* for async buffered IO */ > - randomized_struct_fields_end > }; Now that I've read the thread ... If we care about struct size on 32-bit, we should fit something into the 32-bit hole before the 64-bit loff_t (assuming at least some 32-bit arches want loff_t to be 64-bit aligned; I thik x86 doesn't?) Easiest seems to be to put ki_complete before ki_pos?
On Sun, Aug 14, 2022 at 11:35:35AM +0100, Matthew Wilcox wrote: > On Fri, Aug 12, 2022 at 03:56:33PM -0700, Keith Busch wrote: > > struct kiocb { > > struct file *ki_filp; > > - > > - /* The 'ki_filp' pointer is shared in a union for aio */ > > - randomized_struct_fields_start > > - > > loff_t ki_pos; > > void (*ki_complete)(struct kiocb *iocb, long ret); > > void *private; > > int ki_flags; > > u16 ki_ioprio; /* See linux/ioprio.h */ > > struct wait_page_queue *ki_waitq; /* for async buffered IO */ > > - randomized_struct_fields_end > > }; > > Now that I've read the thread ... > > If we care about struct size on 32-bit, we should fit something into > the 32-bit hole before the 64-bit loff_t (assuming at least some 32-bit > arches want loff_t to be 64-bit aligned; I thik x86 doesn't?) > Easiest seems to be to put ki_complete before ki_pos? Yes, that would be a better compact arrangment. The immediate need was just to correct io_uring's max command struct size. The 32-bit verison isn't near the limit, so no additional optimizations were done in this patch.
diff --git a/include/linux/fs.h b/include/linux/fs.h index 5113f65c786f..9eced4cc286e 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -340,17 +340,12 @@ enum rw_hint { struct kiocb { struct file *ki_filp; - - /* The 'ki_filp' pointer is shared in a union for aio */ - randomized_struct_fields_start - loff_t ki_pos; void (*ki_complete)(struct kiocb *iocb, long ret); void *private; int ki_flags; u16 ki_ioprio; /* See linux/ioprio.h */ struct wait_page_queue *ki_waitq; /* for async buffered IO */ - randomized_struct_fields_end }; static inline bool is_sync_kiocb(struct kiocb *kiocb)