[PATCH] sparse: use force attribute for __kernel_rwf_t casts

[PATCH] sparse: use force attribute for __kernel_rwf_t casts

[Vasily Averin]

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 <vvs@openvz.org>
---
 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

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

[Jens Axboe]

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

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

[Christoph Hellwig]

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 {

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

[Vasily Averin]

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 <vvs@openvz.org>
---
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

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

[Christoph Hellwig]

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.

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

[Jens Axboe]

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

[PATCH v3] io_uring: fix incorrect __kernel_rwf_t cast

[Vasily Averin]

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 <vvs@openvz.org>
---
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

Re: [PATCH v3] io_uring: fix incorrect __kernel_rwf_t cast

[Jens Axboe]

On Thu, 19 May 2022 17:30:49 +0300, Vasily Averin wrote:
> 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
> 
> [...]

Applied, thanks!

[1/1] io_uring: fix incorrect __kernel_rwf_t cast
      commit: 0e7579ca732a39cc377e17509dda9bfc4f6ba78e

Best regards,
-- 
Jens Axboe