On Mon, Apr 25, 2022 at 8:19 AM Jann Horn <jannh@xxxxxxxxxx> wrote: > > On Mon, Apr 25, 2022 at 5:13 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote: > > On Mon, Apr 25, 2022 at 8:01 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > > > > > On Mon, 25 Apr 2022 16:49:34 +0200 Jann Horn wrote: > > > > > Doesn't mean we should make it legal. We can add a warning to catch > > > > > abuses. > > > > > > > > That was the idea with > > > > https://lore.kernel.org/netdev/20220128014303.2334568-1-jannh@xxxxxxxxxx/, > > > > but I didn't get any replies when I asked what the precise semantics > > > > of dev_hold() are supposed to be > > > > (https://lore.kernel.org/netdev/CAG48ez1-OyZETvrYAfaHicYW1LbrQUVp=C0EukSWqZrYMej73w@xxxxxxxxxxxxxx/), > > > > so I don't know how to proceed... > > > > > > Yeah, I think after you pointed out that the netdev per cpu refcounting > > > is fundamentally broken everybody decided to hit themselves with the > > > obliviate spell :S > > > > dev_hold() has been an increment of a refcount, and dev_put() a decrement. > > > > Not sure why it is fundamentally broken. > > Well, it's not quite a refcount. It's a count that can be incremented > and decremented but can't be read while the device is alive, and then > at some point it turns into a count that can be read and decremented > but can't be incremented (see > https://lore.kernel.org/netdev/CAG48ez1-OyZETvrYAfaHicYW1LbrQUVp=C0EukSWqZrYMej73w@xxxxxxxxxxxxxx/). > Normal refcounts allow anyone who is holding a reference to add > another reference. On a live netdev nothing wants to read the 'current refcount'. We basically do not care. > > > There are specific steps at device dismantles making sure no more > > users can dev_hold() > > So you're saying it's intentional that even if you're already holding > a dev_hold() reference, you may not be allowed to call dev_hold() > again? I think you can/should not. We might add a test in dev_hold() and catch offenders. Then add a new api, (dev_hold() is void and can not propagate an error), and eventually fix offenders.
