netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Mina Almasry <almasrymina@google.com>
Cc: "Shakeel Butt" <shakeelb@google.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	kvm@vger.kernel.org, virtualization@lists.linux.dev,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Stefano Garzarella" <sgarzare@redhat.com>,
	"David Howells" <dhowells@redhat.com>,
	"Jason Gunthorpe" <jgg@nvidia.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Yunsheng Lin" <linyunsheng@huawei.com>,
	"Willem de Bruijn" <willemdebruijn.kernel@gmail.com>
Subject: Re: [PATCH net-next v3 2/3] net: introduce abstraction for network memory
Date: Thu, 4 Jan 2024 13:44:24 -0800	[thread overview]
Message-ID: <20240104134424.399fee0a@kernel.org> (raw)
In-Reply-To: <CAHS8izOp_m9SyPjNth-iYBXH2qQQpc9PuZaHbpUL=H0W=CVHgQ@mail.gmail.com>

On Thu, 21 Dec 2023 15:44:22 -0800 Mina Almasry wrote:
> The warning is like so:
> 
> ./include/net/page_pool/helpers.h: In function ‘page_pool_alloc’:
> ./include/linux/stddef.h:8:14: warning: returning ‘void *’ from a
> function with return type ‘netmem_ref’ {aka ‘long unsigned int’} makes
> integer from pointer without a cast [-Wint-conversion]
>     8 | #define NULL ((void *)0)
>       |              ^
> ./include/net/page_pool/helpers.h:132:24: note: in expansion of macro
> ‘NULL’
>   132 |                 return NULL;
>       |                        ^~~~
> 
> And happens in all the code where:
> 
> netmem_ref func()
> {
>     return NULL;
> }
> 
> It's fixable by changing the return to `return (netmem_ref NULL);` or
> `return 0;`, but I feel like netmem_ref should be some type which
> allows a cast from NULL implicitly.

Why do you think we should be able to cast NULL implicitly?
netmem_ref is a handle, it could possibly be some form of 
an ID in the future, rather than a pointer. Or have more low
bits stolen for specific use cases.

unsigned long, and returning 0 as "no handle" makes perfect sense to me.

Note that 0 is a special case, bitwise types are allowed to convert
to 0/bool and 0 is implicitly allowed to become a bitwise type.
This will pass without a warning:

typedef unsigned long __bitwise netmem_ref;

netmem_ref some_code(netmem_ref ref)
{
	// direct test is fine
	if (!ref)
		// 0 "upgrades" without casts
		return 0;
	// 1 does not, we need __force
	return (__force netmem_ref)1 | ref;
}

The __bitwise annotation will make catching people trying
to cast to struct page * trivial.

You seem to be trying hard to make struct netmem a thing.
Perhaps you have a reason I'm not getting?

  reply	other threads:[~2024-01-04 21:44 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-20 21:44 [PATCH net-next v3 0/3] Abstract page from net stack Mina Almasry
2023-12-20 21:45 ` [PATCH net-next v3 1/3] vsock/virtio: use skb_frag_*() helpers Mina Almasry
2023-12-21 17:17   ` Willem de Bruijn
2023-12-21 21:39   ` Shakeel Butt
2024-01-02 10:00   ` Stefano Garzarella
2023-12-20 21:45 ` [PATCH net-next v3 2/3] net: introduce abstraction for network memory Mina Almasry
2023-12-21 23:23   ` Shakeel Butt
2023-12-21 23:44     ` Mina Almasry
2024-01-04 21:44       ` Jakub Kicinski [this message]
2024-01-04 22:15         ` Mina Almasry
2024-01-10 17:50         ` Shakeel Butt
2024-01-11  1:35           ` Jakub Kicinski
2023-12-20 21:45 ` [PATCH net-next v3 3/3] net: add netmem_ref to skb_frag_t Mina Almasry
2023-12-21 17:16   ` Simon Horman
2023-12-21 17:18   ` Willem de Bruijn
2023-12-21 23:27   ` Shakeel Butt
2023-12-22 20:10   ` kernel test robot
2023-12-22 23:39   ` kernel test robot
2023-12-23 11:16   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240104134424.399fee0a@kernel.org \
    --to=kuba@kernel.org \
    --cc=almasrymina@google.com \
    --cc=christian.koenig@amd.com \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=edumazet@google.com \
    --cc=jgg@nvidia.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linyunsheng@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sgarzare@redhat.com \
    --cc=shakeelb@google.com \
    --cc=stefanha@redhat.com \
    --cc=virtualization@lists.linux.dev \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).