2022-05-15 03:39:32

by Vasily Averin

[permalink] [raw]
Subject: [PATCH] sparse: use force attribute for __kernel_rwf_t casts

Fixes sparse warnings:
fs/io_uring.c: note: in included file (through include/trace/perf.h,
include/trace/define_trace.h, include/trace/events/io_uring.h):
./include/trace/events/io_uring.h:488:1: sparse:
warning: incorrect type in assignment (different base types)
expected unsigned int [usertype] op_flags
got restricted __kernel_rwf_t const [usertype] rw_flags
fs/io_uring.c:3164:23: sparse:
warning: incorrect type in assignment (different base types)
expected unsigned int [usertype] flags
got restricted __kernel_rwf_t
fs/io_uring.c:3769:48: sparse:
warning: incorrect type in argument 2 (different base types)
expected restricted __kernel_rwf_t [usertype] flags
got unsigned int [usertype] flags

__kernel_rwf_t type is bitwise and requires __force attribute for casts

Signed-off-by: Vasily Averin <[email protected]>
---
fs/io_uring.c | 4 ++--
include/trace/events/io_uring.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 91de361ea9ab..5ca4a6e91884 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3161,7 +3161,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
req->imu = NULL;
req->rw.addr = READ_ONCE(sqe->addr);
req->rw.len = READ_ONCE(sqe->len);
- req->rw.flags = READ_ONCE(sqe->rw_flags);
+ req->rw.flags = (__force u32)READ_ONCE(sqe->rw_flags);
req->buf_index = READ_ONCE(sqe->buf_index);
return 0;
}
@@ -3766,7 +3766,7 @@ static int io_rw_init_file(struct io_kiocb *req, fmode_t mode)
req->flags |= io_file_get_flags(file) << REQ_F_SUPPORT_NOWAIT_BIT;

kiocb->ki_flags = iocb_flags(file);
- ret = kiocb_set_rw_flags(kiocb, req->rw.flags);
+ ret = kiocb_set_rw_flags(kiocb, (__force rwf_t)req->rw.flags);
if (unlikely(ret))
return ret;

diff --git a/include/trace/events/io_uring.h b/include/trace/events/io_uring.h
index cddf5b6fbeb4..df4b89a79764 100644
--- a/include/trace/events/io_uring.h
+++ b/include/trace/events/io_uring.h
@@ -520,7 +520,7 @@ TRACE_EVENT(io_uring_req_failed,
__entry->off = sqe->off;
__entry->addr = sqe->addr;
__entry->len = sqe->len;
- __entry->op_flags = sqe->rw_flags;
+ __entry->op_flags = (__force u32)sqe->rw_flags;
__entry->buf_index = sqe->buf_index;
__entry->personality = sqe->personality;
__entry->file_index = sqe->file_index;
--
2.31.1



2022-05-15 18:42:57

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] sparse: use force attribute for __kernel_rwf_t casts

On 5/14/22 10:23 AM, Vasily Averin wrote:
> Fixes sparse warnings:
> fs/io_uring.c: note: in included file (through include/trace/perf.h,
> include/trace/define_trace.h, include/trace/events/io_uring.h):
> ./include/trace/events/io_uring.h:488:1: sparse:
> warning: incorrect type in assignment (different base types)
> expected unsigned int [usertype] op_flags
> got restricted __kernel_rwf_t const [usertype] rw_flags
> fs/io_uring.c:3164:23: sparse:
> warning: incorrect type in assignment (different base types)
> expected unsigned int [usertype] flags
> got restricted __kernel_rwf_t
> fs/io_uring.c:3769:48: sparse:
> warning: incorrect type in argument 2 (different base types)
> expected restricted __kernel_rwf_t [usertype] flags
> got unsigned int [usertype] flags

Hand applied for 5.19, didn't apply directly. At some point it'd be nice
to do the poll ones as they are the biggest issue wrt sparse, but
honestly I've never really been very motivated to fix those sparse
warnings as it would just be an unnecessary pain for backporting
patches.

--
Jens Axboe


2022-05-16 10:51:21

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] sparse: use force attribute for __kernel_rwf_t casts

Please stop sprinkling random __force casts. 95% of them are simplify
wrong, and the others need to go into properly documented helpers.

The right fixes here are thing like:

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 4479013854d20..a5d8b5109d3a7 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -585,7 +585,7 @@ struct io_rw {
struct kiocb kiocb;
u64 addr;
u32 len;
- u32 flags;
+ rwf_t flags;
};

struct io_connect {

2022-05-18 09:29:52

by Vasily Averin

[permalink] [raw]
Subject: [PATCH v2] io_uring: fix sparce warnings about __kernel_rwf_t casts

sparse generates follwong warnings:
fs/io_uring.c: note: in included file (through include/trace/perf.h,
include/trace/define_trace.h, include/trace/events/io_uring.h):
./include/trace/events/io_uring.h:488:1: sparse:
warning: incorrect type in assignment (different base types)
expected unsigned int [usertype] op_flags
got restricted __kernel_rwf_t const [usertype] rw_flags
fs/io_uring.c:3164:23: sparse:
warning: incorrect type in assignment (different base types)
expected unsigned int [usertype] flags
got restricted __kernel_rwf_t
fs/io_uring.c:3769:48: sparse:
warning: incorrect type in argument 2 (different base types)
expected restricted __kernel_rwf_t [usertype] flags
got unsigned int [usertype] flags

__kernel_rwf_t type is bitwise and requires __force attribute for casts.

To fix the warnings, the patch changes the type of fields in the
corresponding structures: poll32_events and rw_flags are neighbours
in the same union.

Signed-off-by: Vasily Averin <[email protected]>
---
v2: updated according to comments by Christoph Hellwig
---
fs/io_uring.c | 2 +-
include/trace/events/io_uring.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 91de361ea9ab..e2a40c58654c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -585,7 +585,7 @@ struct io_rw {
struct kiocb kiocb;
u64 addr;
u32 len;
- u32 flags;
+ rwf_t flags;
};

struct io_connect {
diff --git a/include/trace/events/io_uring.h b/include/trace/events/io_uring.h
index cddf5b6fbeb4..34839f30caee 100644
--- a/include/trace/events/io_uring.h
+++ b/include/trace/events/io_uring.h
@@ -520,7 +520,7 @@ TRACE_EVENT(io_uring_req_failed,
__entry->off = sqe->off;
__entry->addr = sqe->addr;
__entry->len = sqe->len;
- __entry->op_flags = sqe->rw_flags;
+ __entry->op_flags = sqe->poll32_events;
__entry->buf_index = sqe->buf_index;
__entry->personality = sqe->personality;
__entry->file_index = sqe->file_index;
--
2.31.1


2022-05-19 15:05:44

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH v2] io_uring: fix sparce warnings about __kernel_rwf_t casts

On 5/19/22 1:03 AM, Christoph Hellwig wrote:
>> index cddf5b6fbeb4..34839f30caee 100644
>> --- a/include/trace/events/io_uring.h
>> +++ b/include/trace/events/io_uring.h
>> @@ -520,7 +520,7 @@ TRACE_EVENT(io_uring_req_failed,
>> __entry->off = sqe->off;
>> __entry->addr = sqe->addr;
>> __entry->len = sqe->len;
>> - __entry->op_flags = sqe->rw_flags;
>> + __entry->op_flags = sqe->poll32_events;
>> __entry->buf_index = sqe->buf_index;
>> __entry->personality = sqe->personality;
>> __entry->file_index = sqe->file_index;
>
> For which I did not see a warning even if it looks real to me.
> But this union with basically a lot of __u32s here looks pretty
> strange to start with to me.

Vasily, if this is still causing unnecessary sparse spews, please send a
revised one against the current branch.

The union is the per-op flags. The requests that use them have a member
in there, either one can be used for printing it obviously.

--
Jens Axboe


2022-05-19 17:52:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2] io_uring: fix sparce warnings about __kernel_rwf_t casts

On Wed, May 18, 2022 at 12:20:46PM +0300, Vasily Averin wrote:
> __kernel_rwf_t type is bitwise and requires __force attribute for casts.
>
> To fix the warnings, the patch changes the type of fields in the
> corresponding structures: poll32_events and rw_flags are neighbours
> in the same union.

Jens actually picked up a series from me that picked up most of this,
except for this hunk:

> index cddf5b6fbeb4..34839f30caee 100644
> --- a/include/trace/events/io_uring.h
> +++ b/include/trace/events/io_uring.h
> @@ -520,7 +520,7 @@ TRACE_EVENT(io_uring_req_failed,
> __entry->off = sqe->off;
> __entry->addr = sqe->addr;
> __entry->len = sqe->len;
> - __entry->op_flags = sqe->rw_flags;
> + __entry->op_flags = sqe->poll32_events;
> __entry->buf_index = sqe->buf_index;
> __entry->personality = sqe->personality;
> __entry->file_index = sqe->file_index;

For which I did not see a warning even if it looks real to me.
But this union with basically a lot of __u32s here looks pretty
strange to start with to me.

2022-05-20 18:44:09

by Vasily Averin

[permalink] [raw]
Subject: [PATCH v3] io_uring: fix incorrect __kernel_rwf_t cast

Currently 'make C=1 fs/io_uring.o' generates sparse warning:

CHECK fs/io_uring.c
fs/io_uring.c: note: in included file (through
include/trace/trace_events.h, include/trace/define_trace.h, i
nclude/trace/events/io_uring.h):
./include/trace/events/io_uring.h:488:1:
warning: incorrect type in assignment (different base types)
expected unsigned int [usertype] op_flags
got restricted __kernel_rwf_t const [usertype] rw_flags

This happen on cast of sqe->rw_flags which is defined as __kernel_rwf_t,
this type is bitwise and requires __force attribute for any casts.
However rw_flags is a member of the union, and its access can be safely
replaced by using of its neighbours, so let's use poll32_events to fix
the sparse warning.

Signed-off-by: Vasily Averin <[email protected]>
---
v3:
1) fix only hunk in TRACE_EVENT(io_uring_req_failed),
rest ones was fixed by Christoph Hellwig already.
2) updated patch description

v2: updated according to comments by Christoph Hellwig

---
include/trace/events/io_uring.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/trace/events/io_uring.h b/include/trace/events/io_uring.h
index 80d2588a090c..6ba87a290a24 100644
--- a/include/trace/events/io_uring.h
+++ b/include/trace/events/io_uring.h
@@ -520,7 +520,7 @@ TRACE_EVENT(io_uring_req_failed,
__entry->off = sqe->off;
__entry->addr = sqe->addr;
__entry->len = sqe->len;
- __entry->op_flags = sqe->rw_flags;
+ __entry->op_flags = sqe->poll32_events;
__entry->buf_index = sqe->buf_index;
__entry->personality = sqe->personality;
__entry->file_index = sqe->file_index;
--
2.31.1