All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Abhinav Singh <singhabhinav9051571833@gmail.com>,
	akpm@linux-foundation.org, brauner@kernel.org, surenb@google.com,
	mst@redhat.com, michael.christie@oracle.com, mjguzik@gmail.com,
	npiggin@gmail.com, peterz@infradead.org,
	paulmck <paulmck@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH v4] Fixing directly deferencing a __rcu pointer warning
Date: Sun, 12 Nov 2023 14:56:05 -0500	[thread overview]
Message-ID: <dbe62cc9-8354-463f-b398-8e80ca17e556@efficios.com> (raw)
In-Reply-To: <20231112193055.2072382-1-singhabhinav9051571833@gmail.com>

On 2023-11-12 14:30, Abhinav Singh wrote:
> This patch fixes the sparse warning with this message
> "dereference of noderef expression" , in this context
> it means about directly dereferencing a pointer tagged
> with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> function rcu_dereference(...) to use when dereferencing a __rcu
> pointer inside rcu read side critical sections. This function
> returns the non __rcu tagged pointer which can be dereferenced
> just like a normal pointer.
> 
> I tested with `lockdep` enabled, with these config options
> `CONFIG_PROVE_RCU` and `CONFIG_PROVE_LOCKING` enabled and it
> booted just fine. To confirm if lockdep was really enabled
> I found these paths inside the qemu virtual envirnoment.

I did not see actions taken nor answer on those comments:

https://lore.kernel.org/all/CAGudoHEfjSAim6Hh-qYPY+qi8nbLx7J3YdpGgFwSvD7xbeYR3w@mail.gmail.com/

Basically, the missing annotation here can be either:

- A missing rcu_dereference, if the intent is to use the pointer while protecting
   with with a read-side critical section,
- A missing rcu_dereference_protected(), if the use of the pointer is protected by
   a lock.

I don't really care if rcu_dereference happens to work in testing or not. _If_ the
intended design is that this rcu pointer is protected by a lock, or being used before
becoming published elsewhere, then using rcu_dereference to silence the warning is
wrong. (note: I did not do a review of the affected code, but I would expect the
commit message to take care of going through this level of detail)

And the fact that this aspect of the feedback has been hidden under the rug worries
me.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com


  reply	other threads:[~2023-11-12 19:55 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-25 22:28 [PATCH] Fixing warning of directly dereferencing __rcu tagged Abhinav Singh
2023-10-25 22:28 ` Abhinav Singh
2023-10-25 22:38 ` Andrew Morton
2023-10-25 22:38   ` Andrew Morton
2023-10-25 23:27   ` Abhinav Singh
2023-10-25 23:27     ` Abhinav Singh
2023-10-25 23:50     ` Andrew Morton
2023-10-25 23:50       ` Andrew Morton
2023-10-26 12:16       ` [PATCH v2] Fixing directly deferencing a __rcu pointer warning Abhinav Singh
2023-10-26 12:16         ` Abhinav Singh
2023-10-26 13:14         ` Michael S. Tsirkin
2023-10-26 13:14           ` Michael S. Tsirkin
2023-10-26 14:06           ` Mateusz Guzik
2023-10-26 14:06             ` Mateusz Guzik
2023-10-26 14:51             ` Mathieu Desnoyers
2023-10-26 14:51               ` Mathieu Desnoyers
2023-10-26 15:07               ` Mateusz Guzik
2023-10-26 15:07                 ` Mateusz Guzik
2023-10-26 15:17             ` Michael S. Tsirkin
2023-10-26 15:17               ` Michael S. Tsirkin
2023-10-26 15:37               ` Abhinav Singh
2023-10-26 15:37                 ` Abhinav Singh
2023-10-26 15:47                 ` Michael S. Tsirkin
2023-10-26 15:47                   ` Michael S. Tsirkin
2023-10-27  6:37                   ` [PATCH v3] " Abhinav Singh
2023-10-27  6:37                     ` Abhinav Singh
2023-10-27  6:41                     ` Abhinav Singh
2023-10-27  6:41                       ` Abhinav Singh
2023-10-27  7:00                       ` Michael S. Tsirkin
2023-10-27  7:00                         ` Michael S. Tsirkin
2023-10-27  6:50                     ` Michael S. Tsirkin
2023-10-27  6:50                       ` Michael S. Tsirkin
2023-10-26 12:18       ` [PATCH] Fixing warning of directly dereferencing __rcu tagged Abhinav Singh
2023-10-26 12:18         ` Abhinav Singh
2023-10-26 12:27       ` [PATCH] Fixing directly deferencing a __rcu pointer warning Abhinav Singh
2023-10-26 12:27         ` Abhinav Singh
2023-10-26 15:03         ` Peter Zijlstra
2023-10-26 15:03           ` Peter Zijlstra
2023-10-27 12:22           ` Oleg Nesterov
2023-10-27 12:22             ` Oleg Nesterov
2023-10-28 10:22             ` [PATCH v3] " Abhinav Singh
2023-10-28 10:22               ` Abhinav Singh
2023-10-28 10:31               ` Abhinav Singh
2023-10-28 10:31                 ` Abhinav Singh
2023-10-28 12:20               ` Michael S. Tsirkin
2023-10-28 12:20                 ` Michael S. Tsirkin
2023-10-28 20:43                 ` Abhinav Singh
2023-10-28 20:43                   ` Abhinav Singh
2023-10-28 20:46                   ` Abhinav Singh
2023-10-28 20:46                     ` Abhinav Singh
2023-11-03  6:30                     ` kernel test robot
2023-11-03  6:30                       ` kernel test robot
2023-11-12 19:30                       ` [PATCH v4] " Abhinav Singh
2023-11-12 19:56                         ` Mathieu Desnoyers [this message]
2023-11-12 19:58                         ` Peter Zijlstra
2023-11-12 20:18                           ` Abhinav Singh

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=dbe62cc9-8354-463f-b398-8e80ca17e556@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.christie@oracle.com \
    --cc=mjguzik@gmail.com \
    --cc=mst@redhat.com \
    --cc=npiggin@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=singhabhinav9051571833@gmail.com \
    --cc=surenb@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.