| Message ID | 20190501042831.5313-2-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blktrace: add blktrace extension support | expand |
On Tue, 2019-04-30 at 21:28 -0700, Chaitanya Kulkarni wrote: > -#define BLKTRACESETUP32 _IOWR(0x12, 115, struct compat_blk_user_trace_setup) > + > +/* XXX: temp work around for RFC */ > +#define BLKTRACESETUP32 _IOWR(0x13, 115, struct compat_blk_user_trace_setup) This change breaks user space so this change is not acceptable. I think you want to introduce a new ioctl instead of modifying an existing ioctl. Additionally, have you considered to split the blktrace_api.h header file into two header files: one with kernel-internal definitions and a second one with definitions that are shared with user space (include/uapi/...)? Thanks, Bart.
On 5/1/19 8:48 AM, Bart Van Assche wrote: > On Tue, 2019-04-30 at 21:28 -0700, Chaitanya Kulkarni wrote: >> -#define BLKTRACESETUP32 _IOWR(0x12, 115, struct compat_blk_user_trace_setup) >> + >> +/* XXX: temp work around for RFC */ >> +#define BLKTRACESETUP32 _IOWR(0x13, 115, struct compat_blk_user_trace_setup) > > This change breaks user space so this change is not acceptable. I think you > want to introduce a new ioctl instead of modifying an existing ioctl. > Additionally, have you considered to split the blktrace_api.h header file > into two header files: one with kernel-internal definitions and a second one > with definitions that are shared with user space (include/uapi/...)? > > Thanks, > > Bart. > I want to avoid modifying an existing IOCTL, I'll add a new ioctl and update the tools to use the extension IOCTL and split the header file also. Also I found that user space tools have replicated BLK_XX_XXX definitions, will be okay to keep all those in one place and include those from the appropriate header file ?
On Thu, 2019-05-02 at 03:43 +0000, Chaitanya Kulkarni wrote: > On 5/1/19 8:48 AM, Bart Van Assche wrote: > > On Tue, 2019-04-30 at 21:28 -0700, Chaitanya Kulkarni wrote: > > > -#define BLKTRACESETUP32 _IOWR(0x12, 115, struct compat_blk_user_trace_setup) > > > + > > > +/* XXX: temp work around for RFC */ > > > +#define BLKTRACESETUP32 _IOWR(0x13, 115, struct compat_blk_user_trace_setup) > > > > This change breaks user space so this change is not acceptable. I think you > > want to introduce a new ioctl instead of modifying an existing ioctl. > > Additionally, have you considered to split the blktrace_api.h header file > > into two header files: one with kernel-internal definitions and a second one > > with definitions that are shared with user space (include/uapi/...)? > > I want to avoid modifying an existing IOCTL, I'll add a new ioctl and > update the tools to use the extension IOCTL and split the header file > also. Also I found that user space tools have replicated BLK_XX_XXX > definitions, will be okay to keep all those in one place and include > those from the appropriate header file ? Hi Chaitanya, I think all definitions that are relevant for the user space blktrace tool should be moved into a header file under include/uapi/linux. I'm not sure what the best strategy is to include that header file in the blktrace tool. Another project that interfaces with the kernel (rdma-core; see also https://github.com/linux-rdma/rdma-core/) periodically copies kernel header files into its own source code repository. Bart.
diff --git a/include/linux/blktrace_api.h b/include/linux/blktrace_api.h index 7bb2d8de9f30..403d4cfc6a52 100644 --- a/include/linux/blktrace_api.h +++ b/include/linux/blktrace_api.h @@ -17,7 +17,11 @@ struct blk_trace { struct rchan *rchan; unsigned long __percpu *sequence; unsigned char __percpu *msg_data; +#ifdef CONFIG_BLKTRACE_EXT + u64 act_mask; +#else u16 act_mask; +#endif /* CONFIG_BLKTRACE_EXT */ u64 start_lba; u64 end_lba; u32 pid; @@ -101,14 +105,20 @@ static inline int blk_trace_init_sysfs(struct device *dev) struct compat_blk_user_trace_setup { char name[BLKTRACE_BDEV_SIZE]; +#ifdef CONFIG_BLKTRACE_EXT + u64 act_mask; +#else u16 act_mask; +#endif /* CONFIG_BLKTRACE_EXT */ u32 buf_size; u32 buf_nr; compat_u64 start_lba; compat_u64 end_lba; u32 pid; }; -#define BLKTRACESETUP32 _IOWR(0x12, 115, struct compat_blk_user_trace_setup) + +/* XXX: temp work around for RFC */ +#define BLKTRACESETUP32 _IOWR(0x13, 115, struct compat_blk_user_trace_setup) #endif
This patch adds the blktrace extension support where we increase the size of action mask so that it can store more actions. Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- include/linux/blktrace_api.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)