[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230630091442.172ec67f@kernel.org>
Date: Fri, 30 Jun 2023 09:14:42 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Nathan Chancellor <nathan@...nel.org>
Cc: David Howells <dhowells@...hat.com>,
Aurelien Aptel <aaptel@...dia.com>, netdev@...r.kernel.org,
Alexander Duyck <alexander.duyck@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
David Ahern <dsahern@...nel.org>,
Matthew Wilcox <willy@...radead.org>,
Jens Axboe <axboe@...nel.dk>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Sagi Grimberg <sagi@...mberg.me>,
Willem de Bruijn <willemb@...gle.com>,
Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...com>,
Christoph Hellwig <hch@....de>,
Chaitanya Kulkarni <kch@...dia.com>,
linux-nvme@...ts.infradead.org, llvm@...ts.linux.dev
Subject: Re: [PATCH net-next v3 10/18] nvme/host: Use
sendmsg(MSG_SPLICE_PAGES) rather then sendpage
On Fri, 30 Jun 2023 09:10:43 -0700 Nathan Chancellor wrote:
> > Let me CC llvm@ in case someone's there is willing to make
> > the compiler warn about this.
>
> Turns out clang already has a warning for this, -Wcomma:
>
> drivers/nvme/host/tcp.c:1017:38: error: possible misuse of comma operator here [-Werror,-Wcomma]
> 1017 | msg.msg_flags &= ~MSG_SPLICE_PAGES,
> | ^
> drivers/nvme/host/tcp.c:1017:4: note: cast expression to void to silence warning
> 1017 | msg.msg_flags &= ~MSG_SPLICE_PAGES,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | (void)( )
> 1 error generated.
>
> Let me do some wider build testing to see if it is viable to turn this
> on for the whole kernel because it seems worth it, at least in this
> case. There are a lot of cases where a warning won't be emitted (see the
> original upstream review for a list: https://reviews.llvm.org/D3976) but
> something is better than nothing, right? :)
Ah, neat. Misleading indentation is another possible angle, I reckon,
but not sure if that's enabled/possible to enable for the entire kernel
either :( We test-build with W=1 in networking, FWIW, so W=1 would be
enough for us.
Powered by blists - more mailing lists