| Message ID | 02d9ec4a10235def0e764ff1f5be881ba12e16e8.1704397858.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/P2PDMA: Fix a sleeping issue in a RCU read section | expand |
On 2024-01-04 12:52, Christophe JAILLET wrote: > It is not allowed to sleep within a RCU read section, so use GFP_ATOMIC > instead of GFP_KERNEL here. > > Fixes: ae21f835a5bd ("PCI/P2PDMA: Finish RCU conversion of pdev->p2pdma") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> This makes sense to me. Though, the use of RCU could probably use a review. Seeing p2pdma is only released through a devm action on the pdev, I would think it shouldn't be needed if we hold a reference to the pdev. Other than that: Reviewed-by: Logan Gunthorpe <logang@deltatee.com> Thanks! Logan
On Thu, Jan 04, 2024 at 08:52:35PM +0100, Christophe JAILLET wrote: > It is not allowed to sleep within a RCU read section, so use GFP_ATOMIC > instead of GFP_KERNEL here. > > Fixes: ae21f835a5bd ("PCI/P2PDMA: Finish RCU conversion of pdev->p2pdma") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > This patch is speculative. > It is based on a discussion related to another patch. See [1]. > > It also matches the doc, IIUC. See [2] > > [1]: https://lore.kernel.org/all/20240104143925.194295-3-alexis.lothore@bootlin.com/ > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/RCU/whatisRCU.rst#n161 Looks good to me. Smatch is supposed to catch this sort of bug but there are some issues with xa_store it's normally holding the the xas_lock() but if you pass a sleeping GFP_ then it drops the lock in __xas_nomem(). Sort of tricky. So right now all xa_store() stuff triggers a false positive but tomorrows version of Smatch will just miss this bug. regards, dan carpenter
On Thu, Jan 04, 2024 at 08:52:35PM +0100, Christophe JAILLET wrote: > It is not allowed to sleep within a RCU read section, so use GFP_ATOMIC > instead of GFP_KERNEL here. > > Fixes: ae21f835a5bd ("PCI/P2PDMA: Finish RCU conversion of pdev->p2pdma") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Applied with Logan's Reviewed-by to pci/p2pdma for v6.9, thanks! > --- > This patch is speculative. > It is based on a discussion related to another patch. See [1]. > > It also matches the doc, IIUC. See [2] > > [1]: https://lore.kernel.org/all/20240104143925.194295-3-alexis.lothore@bootlin.com/ > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/RCU/whatisRCU.rst#n161 > --- > drivers/pci/p2pdma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index 0c361561b855..4f47a13cb500 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c > @@ -661,7 +661,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client, > p2pdma = rcu_dereference(provider->p2pdma); > if (p2pdma) > xa_store(&p2pdma->map_types, map_types_idx(client), > - xa_mk_value(map_type), GFP_KERNEL); > + xa_mk_value(map_type), GFP_ATOMIC); > rcu_read_unlock(); > return map_type; > } > -- > 2.34.1 >
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index 0c361561b855..4f47a13cb500 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -661,7 +661,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client, p2pdma = rcu_dereference(provider->p2pdma); if (p2pdma) xa_store(&p2pdma->map_types, map_types_idx(client), - xa_mk_value(map_type), GFP_KERNEL); + xa_mk_value(map_type), GFP_ATOMIC); rcu_read_unlock(); return map_type; }
It is not allowed to sleep within a RCU read section, so use GFP_ATOMIC instead of GFP_KERNEL here. Fixes: ae21f835a5bd ("PCI/P2PDMA: Finish RCU conversion of pdev->p2pdma") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- This patch is speculative. It is based on a discussion related to another patch. See [1]. It also matches the doc, IIUC. See [2] [1]: https://lore.kernel.org/all/20240104143925.194295-3-alexis.lothore@bootlin.com/ [2]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/RCU/whatisRCU.rst#n161 --- drivers/pci/p2pdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)