| Message ID | 20221102082503.32236-1-korantwork@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] io_uring: fix two assignments in if conditions | expand |
On 11/2/22 2:25 AM, korantwork@gmail.com wrote: > From: Xinghui Li <korantli@tencent.com> > > Fixs two error: > > "ERROR: do not use assignment in if condition > 130: FILE: io_uring/net.c:130: > + if (!(issue_flags & IO_URING_F_UNLOCKED) && > > ERROR: do not use assignment in if condition > 599: FILE: io_uring/poll.c:599: > + } else if (!(issue_flags & IO_URING_F_UNLOCKED) &&" > reported by checkpatch.pl in net.c and poll.c . I'm not super excited about this patch as the previous one wasn't even compiled? Did you test this one, compile and runtime?
在 2022/11/2 22:19，“Jens Axboe”<axboe@kernel.dk> 写入: > On 11/2/22 2:25 AM, korantwork@gmail.com wrote: > > From: Xinghui Li <korantli@tencent.com> > > > > Fixs two error: > > > > "ERROR: do not use assignment in if condition > > 130: FILE: io_uring/net.c:130: > > + if (!(issue_flags & IO_URING_F_UNLOCKED) && > > > > ERROR: do not use assignment in if condition > > 599: FILE: io_uring/poll.c:599: > > + } else if (!(issue_flags & IO_URING_F_UNLOCKED) &&" > > reported by checkpatch.pl in net.c and poll.c . > > I'm not super excited about this patch as the previous one > wasn't even compiled? Did you test this one, compile and runtime? > First of all, I'm really really really SORRY for the last stupid warning and my careless. But I did make kernel and install it in my device. Also, I did the fio test with polling. However, there is no warning be reported by 'make' command. But when I check the patch code with 'smatch', I did find that warning. I realized how stupid that mistake was.... So, in addition to compiling and installing this kernel, I also checked the code using the static analysis tool this time. There is no new error or warning. " make -j64 CHECK="~/workspace/smatch/smatch" C=1 DESCEND objtool CALL scripts/checksyscalls.sh CC io_uring/net.o CC io_uring/poll.o CHECK io_uring/poll.c io_uring/poll.c: note: in included file (through io_uring/io_uring.h): io_uring/slist.h:138:29: warning: no newline at end of file CHECK io_uring/net.c io_uring/poll.c: note: in included file (through include/trace/events/io_uring.h): ./include/linux/io_uring_types.h:151:37: warning: array of flexible structures io_uring/net.c: note: in included file (through io_uring/io_uring.h): io_uring/slist.h:138:29: warning: no newline at end of file io_uring/net.c: note: in included file (through io_uring/io_uring.h): ./include/linux/io_uring_types.h:151:37: warning: array of flexible structures ... BUILD arch/x86/boot/bzImage Kernel: arch/x86/boot/bzImage is ready (#10) " Again, I am ashamed of the previous mistake. And I totally understand if you reject this one.
On 11/2/22 8:50 PM, Xinghui Li wrote: > > > ?? 2022/11/2 22:19??Jens Axboe?<axboe@kernel.dk> ??: > >> On 11/2/22 2:25 AM, korantwork@gmail.com wrote: >> > From: Xinghui Li <korantli@tencent.com> >> > >> > Fixs two error: >> > >> > "ERROR: do not use assignment in if condition >> > 130: FILE: io_uring/net.c:130: >> > + if (!(issue_flags & IO_URING_F_UNLOCKED) && >> > >> > ERROR: do not use assignment in if condition >> > 599: FILE: io_uring/poll.c:599: >> > + } else if (!(issue_flags & IO_URING_F_UNLOCKED) &&" >> > reported by checkpatch.pl in net.c and poll.c . >> >> I'm not super excited about this patch as the previous one >> wasn't even compiled? Did you test this one, compile and runtime? >> > > First of all, I'm really really really SORRY for the last stupid > warning and my careless. But I did make kernel and install it in my > device. Also, I did the fio test with polling. However, there is no > warning be reported by 'make' command. But when I check the patch code > with 'smatch', I did find that warning. I realized how stupid that > mistake was.... So, in addition to compiling and installing this > kernel, I also checked the code using the static analysis tool this > time. There is no new error or warning. > " > make -j64 CHECK="~/workspace/smatch/smatch" C=1 > DESCEND objtool > CALL scripts/checksyscalls.sh > CC io_uring/net.o > CC io_uring/poll.o > CHECK io_uring/poll.c > io_uring/poll.c: note: in included file (through io_uring/io_uring.h): > io_uring/slist.h:138:29: warning: no newline at end of file > CHECK io_uring/net.c > io_uring/poll.c: note: in included file (through include/trace/events/io_uring.h): > ./include/linux/io_uring_types.h:151:37: warning: array of flexible structures > io_uring/net.c: note: in included file (through io_uring/io_uring.h): > io_uring/slist.h:138:29: warning: no newline at end of file > io_uring/net.c: note: in included file (through io_uring/io_uring.h): > ./include/linux/io_uring_types.h:151:37: warning: array of flexible structures > ... > BUILD arch/x86/boot/bzImage > Kernel: arch/x86/boot/bzImage is ready (#10) > " > > Again, I am ashamed of the previous mistake. And I totally understand > if you reject this one. All good, thanks for doing the due diligence this time around for v2. I'll get it applied for 6.2, thanks.
On Wed, 2 Nov 2022 16:25:03 +0800, korantwork@gmail.com wrote: > From: Xinghui Li <korantli@tencent.com> > > Fixs two error: > > "ERROR: do not use assignment in if condition > 130: FILE: io_uring/net.c:130: > + if (!(issue_flags & IO_URING_F_UNLOCKED) && > > [...] Applied, thanks! [1/1] io_uring: fix two assignments in if conditions commit: c71afa164bfe91f0825c359940899e539abcb6b7 Best regards,
diff --git a/io_uring/net.c b/io_uring/net.c index 15dea91625e2..cbd655c88499 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -127,13 +127,15 @@ static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req, struct io_cache_entry *entry; struct io_async_msghdr *hdr; - if (!(issue_flags & IO_URING_F_UNLOCKED) && - (entry = io_alloc_cache_get(&ctx->netmsg_cache)) != NULL) { - hdr = container_of(entry, struct io_async_msghdr, cache); - hdr->free_iov = NULL; - req->flags |= REQ_F_ASYNC_DATA; - req->async_data = hdr; - return hdr; + if (!(issue_flags & IO_URING_F_UNLOCKED)) { + entry = io_alloc_cache_get(&ctx->netmsg_cache); + if (entry != NULL) { + hdr = container_of(entry, struct io_async_msghdr, cache); + hdr->free_iov = NULL; + req->flags |= REQ_F_ASYNC_DATA; + req->async_data = hdr; + return hdr; + } } if (!io_alloc_async_data(req)) { diff --git a/io_uring/poll.c b/io_uring/poll.c index 0d9f49c575e0..4b3b4441e9ca 100644 --- a/io_uring/poll.c +++ b/io_uring/poll.c @@ -596,10 +596,13 @@ static struct async_poll *io_req_alloc_apoll(struct io_kiocb *req, if (req->flags & REQ_F_POLLED) { apoll = req->apoll; kfree(apoll->double_poll); - } else if (!(issue_flags & IO_URING_F_UNLOCKED) && - (entry = io_alloc_cache_get(&ctx->apoll_cache)) != NULL) { + } else if (!(issue_flags & IO_URING_F_UNLOCKED)) { + entry = io_alloc_cache_get(&ctx->apoll_cache); + if (entry == NULL) + goto out; apoll = container_of(entry, struct async_poll, cache); } else { +out: apoll = kmalloc(sizeof(*apoll), GFP_ATOMIC); if (unlikely(!apoll)) return NULL;