* [PATCH] RDMA: During rereg_mr ensure that REREG_ACCESS is compatible
@ 2026-06-08 16:44 Jason Gunthorpe
2026-06-08 22:38 ` yanjun.zhu
0 siblings, 1 reply; 3+ messages in thread
From: Jason Gunthorpe @ 2026-06-08 16:44 UTC (permalink / raw)
To: Junxian Huang, Krzysztof Czurylo, linux-rdma, Chengchang Tang,
Tatyana Nikolova, Yishai Hadas, Zhu Yanjun
Cc: Andrew Morton, David Hildenbrand, Leon Romanovsky, patches,
Philip Tsukerman, stable
If IB_MR_REREG_ACCESS changes from RO to RW then the umem has to be
re-evaluated to ensure it is properly pinned as RW. Since the umem is
hidden inside each driver's mr struct add a ib_umem_check_rereg() function
that each driver has to call before processing IB_MR_REREG_ACCESS.
mlx4 has to retain its duplicate ib_access_writable check because it
implements IB_MR_REREG_ACCESS | IB_MR_REREG_TRANS by changing both items
in place sequentially while the MR is live, so it will continue to not
support this combination.
Cc: stable@vger.kernel.org
Fixes: b40656aa7d55 ("RDMA/umem: remove FOLL_FORCE usage")
Reported-by: Philip Tsukerman <philiptsukerman@gmail.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/infiniband/core/umem.c | 16 ++++++++++++++++
drivers/infiniband/hw/hns/hns_roce_mr.c | 4 ++++
drivers/infiniband/hw/irdma/verbs.c | 4 ++++
drivers/infiniband/hw/mlx4/mr.c | 4 ++++
drivers/infiniband/hw/mlx5/mr.c | 4 ++++
drivers/infiniband/sw/rxe/rxe_verbs.c | 5 +++++
include/rdma/ib_umem.h | 8 ++++++++
7 files changed, 45 insertions(+)
diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index 786fa1aa8e552b..4b055712b0d0db 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -332,3 +332,19 @@ int ib_umem_copy_from(void *dst, struct ib_umem *umem, size_t offset,
return 0;
}
EXPORT_SYMBOL(ib_umem_copy_from);
+
+/*
+ * Called during rereg mr if the driver is able to re-use a umem for
+ * IB_MR_REREG_ACCESS.
+ */
+int ib_umem_check_rereg(struct ib_umem *umem, int flags, int new_access_flags)
+{
+ if (!umem)
+ return 0;
+
+ if ((flags & IB_MR_REREG_ACCESS) && !(flags & IB_MR_REREG_TRANS))
+ if (ib_access_writable(new_access_flags) && !umem->writable)
+ return -EACCES;
+ return 0;
+}
+EXPORT_SYMBOL(ib_umem_check_rereg);
diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c b/drivers/infiniband/hw/hns/hns_roce_mr.c
index 896af1828a38de..25bfd3970f5b6e 100644
--- a/drivers/infiniband/hw/hns/hns_roce_mr.c
+++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
@@ -300,6 +300,10 @@ struct ib_mr *hns_roce_rereg_user_mr(struct ib_mr *ibmr, int flags, u64 start,
goto err_out;
}
+ ret = ib_umem_check_rereg(mr->pbl_mtr.umem, flags, mr_access_flags);
+ if (ret)
+ goto err_out;
+
mailbox = hns_roce_alloc_cmd_mailbox(hr_dev);
ret = PTR_ERR_OR_ZERO(mailbox);
if (ret)
diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index 17086048d2d7fc..8cd4275328052e 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -3803,6 +3803,10 @@ static struct ib_mr *irdma_rereg_user_mr(struct ib_mr *ib_mr, int flags,
if (flags & ~(IB_MR_REREG_TRANS | IB_MR_REREG_PD | IB_MR_REREG_ACCESS))
return ERR_PTR(-EOPNOTSUPP);
+ ret = ib_umem_check_rereg(iwmr->region, flags, new_access);
+ if (ret)
+ return ERR_PTR(ret);
+
if (dmabuf_revocable) {
umem_dmabuf = to_ib_umem_dmabuf(iwmr->region);
diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c
index 650b4a9121ff6d..6747bca3067770 100644
--- a/drivers/infiniband/hw/mlx4/mr.c
+++ b/drivers/infiniband/hw/mlx4/mr.c
@@ -209,6 +209,10 @@ struct ib_mr *mlx4_ib_rereg_user_mr(struct ib_mr *mr, int flags, u64 start,
struct mlx4_mpt_entry **pmpt_entry = &mpt_entry;
int err;
+ err = ib_umem_check_rereg(mmr->umem, flags, mr_access_flags);
+ if (err)
+ return ERR_PTR(err);
+
/* Since we synchronize this call and mlx4_ib_dereg_mr via uverbs,
* we assume that the calls can't run concurrently. Otherwise, a
* race exists.
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 3b6da45061a552..fb40b44496f47a 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -1179,6 +1179,10 @@ struct ib_mr *mlx5_ib_rereg_user_mr(struct ib_mr *ib_mr, int flags, u64 start,
if (flags & ~(IB_MR_REREG_TRANS | IB_MR_REREG_PD | IB_MR_REREG_ACCESS))
return ERR_PTR(-EOPNOTSUPP);
+ err = ib_umem_check_rereg(mr->umem, flags, new_access_flags);
+ if (err)
+ return ERR_PTR(err);
+
if (!(flags & IB_MR_REREG_ACCESS))
new_access_flags = mr->access_flags;
if (!(flags & IB_MR_REREG_PD))
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 4d4891dc28846b..4cf04a44189c64 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1319,6 +1319,7 @@ static struct ib_mr *rxe_rereg_user_mr(struct ib_mr *ibmr, int flags,
struct rxe_mr *mr = to_rmr(ibmr);
struct rxe_pd *old_pd = to_rpd(ibmr->pd);
struct rxe_pd *pd = to_rpd(ibpd);
+ int err;
/* for now only support the two easy cases:
* rereg_pd and rereg_access
@@ -1328,6 +1329,10 @@ static struct ib_mr *rxe_rereg_user_mr(struct ib_mr *ibmr, int flags,
return ERR_PTR(-EOPNOTSUPP);
}
+ err = ib_umem_check_rereg(mr->umem, flags, access);
+ if (err)
+ return ERR_PTR(err);
+
if (flags & IB_MR_REREG_PD) {
rxe_put(old_pd);
rxe_get(pd);
diff --git a/include/rdma/ib_umem.h b/include/rdma/ib_umem.h
index 2ad52cc1d52bdd..49172098a8de14 100644
--- a/include/rdma/ib_umem.h
+++ b/include/rdma/ib_umem.h
@@ -156,6 +156,8 @@ void ib_umem_dmabuf_revoke_lock(struct ib_umem_dmabuf *umem_dmabuf);
void ib_umem_dmabuf_revoke_unlock(struct ib_umem_dmabuf *umem_dmabuf);
void ib_umem_dmabuf_revoke(struct ib_umem_dmabuf *umem_dmabuf);
+int ib_umem_check_rereg(struct ib_umem *umem, int flags, int new_access_flags);
+
#else /* CONFIG_INFINIBAND_USER_MEM */
#include <linux/err.h>
@@ -230,5 +232,11 @@ static inline void ib_umem_dmabuf_revoke_lock(struct ib_umem_dmabuf *umem_dmabuf
static inline void ib_umem_dmabuf_revoke_unlock(struct ib_umem_dmabuf *umem_dmabuf) {}
static inline void ib_umem_dmabuf_revoke(struct ib_umem_dmabuf *umem_dmabuf) {}
+static inline int ib_umem_check_rereg(struct ib_umem *umem, int flags,
+ int new_access_flags)
+{
+ return -EOPNOTSUPP;
+}
+
#endif /* CONFIG_INFINIBAND_USER_MEM */
#endif /* IB_UMEM_H */
base-commit: 323c98a4ff06aa28114f2bf658fb43eb3b536bbc
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] RDMA: During rereg_mr ensure that REREG_ACCESS is compatible
2026-06-08 16:44 [PATCH] RDMA: During rereg_mr ensure that REREG_ACCESS is compatible Jason Gunthorpe
@ 2026-06-08 22:38 ` yanjun.zhu
2026-06-08 23:25 ` Jason Gunthorpe
0 siblings, 1 reply; 3+ messages in thread
From: yanjun.zhu @ 2026-06-08 22:38 UTC (permalink / raw)
To: Jason Gunthorpe, Junxian Huang, Krzysztof Czurylo, linux-rdma,
Chengchang Tang, Tatyana Nikolova, Yishai Hadas, Zhu Yanjun,
Zhu Yanjun
Cc: Andrew Morton, David Hildenbrand, Leon Romanovsky, patches,
Philip Tsukerman, stable
On 6/8/26 9:44 AM, Jason Gunthorpe wrote:
> If IB_MR_REREG_ACCESS changes from RO to RW then the umem has to be
> re-evaluated to ensure it is properly pinned as RW. Since the umem is
> hidden inside each driver's mr struct add a ib_umem_check_rereg() function
> that each driver has to call before processing IB_MR_REREG_ACCESS.
>
> mlx4 has to retain its duplicate ib_access_writable check because it
> implements IB_MR_REREG_ACCESS | IB_MR_REREG_TRANS by changing both items
> in place sequentially while the MR is live, so it will continue to not
> support this combination.
>
> Cc: stable@vger.kernel.org
> Fixes: b40656aa7d55 ("RDMA/umem: remove FOLL_FORCE usage")
> Reported-by: Philip Tsukerman <philiptsukerman@gmail.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/infiniband/core/umem.c | 16 ++++++++++++++++
> drivers/infiniband/hw/hns/hns_roce_mr.c | 4 ++++
> drivers/infiniband/hw/irdma/verbs.c | 4 ++++
> drivers/infiniband/hw/mlx4/mr.c | 4 ++++
> drivers/infiniband/hw/mlx5/mr.c | 4 ++++
> drivers/infiniband/sw/rxe/rxe_verbs.c | 5 +++++
> include/rdma/ib_umem.h | 8 ++++++++
> 7 files changed, 45 insertions(+)
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 786fa1aa8e552b..4b055712b0d0db 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -332,3 +332,19 @@ int ib_umem_copy_from(void *dst, struct ib_umem *umem, size_t offset,
> return 0;
> }
> EXPORT_SYMBOL(ib_umem_copy_from);
> +
> +/*
> + * Called during rereg mr if the driver is able to re-use a umem for
> + * IB_MR_REREG_ACCESS.
> + */
> +int ib_umem_check_rereg(struct ib_umem *umem, int flags, int new_access_flags)
> +{
> + if (!umem)
> + return 0;
> +
> + if ((flags & IB_MR_REREG_ACCESS) && !(flags & IB_MR_REREG_TRANS))
> + if (ib_access_writable(new_access_flags) && !umem->writable)
> + return -EACCES;
> + return 0;
> +}
> +EXPORT_SYMBOL(ib_umem_check_rereg);
> diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c b/drivers/infiniband/hw/hns/hns_roce_mr.c
> index 896af1828a38de..25bfd3970f5b6e 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_mr.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
> @@ -300,6 +300,10 @@ struct ib_mr *hns_roce_rereg_user_mr(struct ib_mr *ibmr, int flags, u64 start,
> goto err_out;
> }
>
> + ret = ib_umem_check_rereg(mr->pbl_mtr.umem, flags, mr_access_flags);
> + if (ret)
> + goto err_out;
> +
> mailbox = hns_roce_alloc_cmd_mailbox(hr_dev);
> ret = PTR_ERR_OR_ZERO(mailbox);
> if (ret)
> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index 17086048d2d7fc..8cd4275328052e 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c
> @@ -3803,6 +3803,10 @@ static struct ib_mr *irdma_rereg_user_mr(struct ib_mr *ib_mr, int flags,
> if (flags & ~(IB_MR_REREG_TRANS | IB_MR_REREG_PD | IB_MR_REREG_ACCESS))
> return ERR_PTR(-EOPNOTSUPP);
>
> + ret = ib_umem_check_rereg(iwmr->region, flags, new_access);
> + if (ret)
> + return ERR_PTR(ret);
> +
> if (dmabuf_revocable) {
> umem_dmabuf = to_ib_umem_dmabuf(iwmr->region);
>
> diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c
> index 650b4a9121ff6d..6747bca3067770 100644
> --- a/drivers/infiniband/hw/mlx4/mr.c
> +++ b/drivers/infiniband/hw/mlx4/mr.c
> @@ -209,6 +209,10 @@ struct ib_mr *mlx4_ib_rereg_user_mr(struct ib_mr *mr, int flags, u64 start,
> struct mlx4_mpt_entry **pmpt_entry = &mpt_entry;
> int err;
>
> + err = ib_umem_check_rereg(mmr->umem, flags, mr_access_flags);
> + if (err)
> + return ERR_PTR(err);
> +
> /* Since we synchronize this call and mlx4_ib_dereg_mr via uverbs,
> * we assume that the calls can't run concurrently. Otherwise, a
> * race exists.
> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index 3b6da45061a552..fb40b44496f47a 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
> @@ -1179,6 +1179,10 @@ struct ib_mr *mlx5_ib_rereg_user_mr(struct ib_mr *ib_mr, int flags, u64 start,
> if (flags & ~(IB_MR_REREG_TRANS | IB_MR_REREG_PD | IB_MR_REREG_ACCESS))
> return ERR_PTR(-EOPNOTSUPP);
>
> + err = ib_umem_check_rereg(mr->umem, flags, new_access_flags);
> + if (err)
> + return ERR_PTR(err);
> +
> if (!(flags & IB_MR_REREG_ACCESS))
> new_access_flags = mr->access_flags;
> if (!(flags & IB_MR_REREG_PD))
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 4d4891dc28846b..4cf04a44189c64 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -1319,6 +1319,7 @@ static struct ib_mr *rxe_rereg_user_mr(struct ib_mr *ibmr, int flags,
> struct rxe_mr *mr = to_rmr(ibmr);
> struct rxe_pd *old_pd = to_rpd(ibmr->pd);
> struct rxe_pd *pd = to_rpd(ibpd);
> + int err;
>
> /* for now only support the two easy cases:
> * rereg_pd and rereg_access
> @@ -1328,6 +1329,10 @@ static struct ib_mr *rxe_rereg_user_mr(struct ib_mr *ibmr, int flags,
> return ERR_PTR(-EOPNOTSUPP);
> }
>
> + err = ib_umem_check_rereg(mr->umem, flags, access);
> + if (err)
> + return ERR_PTR(err);
> +
Thanks a lot. I am fine with this.
Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>
But I found the following problem. I am not sure if we fix this problem
in this commit or file a new commit.
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c
b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 4d4891dc2884..3b99649c342d 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1319,6 +1319,7 @@ static struct ib_mr *rxe_rereg_user_mr(struct
ib_mr *ibmr, int flags,
struct rxe_mr *mr = to_rmr(ibmr);
struct rxe_pd *old_pd = to_rpd(ibmr->pd);
struct rxe_pd *pd = to_rpd(ibpd);
+ struct ib_pd *old_ibpd;
/* for now only support the two easy cases:
* rereg_pd and rereg_access
@@ -1331,12 +1332,18 @@ static struct ib_mr *rxe_rereg_user_mr(struct
ib_mr *ibmr, int flags,
if (flags & IB_MR_REREG_PD) {
rxe_put(old_pd);
rxe_get(pd);
+ old_ibpd = mr->ibmr.pd;
mr->ibmr.pd = ibpd;
}
if (flags & IB_MR_REREG_ACCESS) {
if (access & ~RXE_ACCESS_SUPPORTED_MR) {
rxe_err_mr(mr, "access = %#x not supported\n",
access);
+ if (flags & IB_MR_REREG_PD) {
+ rxe_get(old_pd);
+ rxe_put(pd);
+ mr->ibmr.pd = old_ibpd;
+ }
return ERR_PTR(-EOPNOTSUPP);
}
mr->access = access;
Zhu Yanjun
> if (flags & IB_MR_REREG_PD) {
> rxe_put(old_pd);
> rxe_get(pd);
> diff --git a/include/rdma/ib_umem.h b/include/rdma/ib_umem.h
> index 2ad52cc1d52bdd..49172098a8de14 100644
> --- a/include/rdma/ib_umem.h
> +++ b/include/rdma/ib_umem.h
> @@ -156,6 +156,8 @@ void ib_umem_dmabuf_revoke_lock(struct ib_umem_dmabuf *umem_dmabuf);
> void ib_umem_dmabuf_revoke_unlock(struct ib_umem_dmabuf *umem_dmabuf);
> void ib_umem_dmabuf_revoke(struct ib_umem_dmabuf *umem_dmabuf);
>
> +int ib_umem_check_rereg(struct ib_umem *umem, int flags, int new_access_flags);
> +
> #else /* CONFIG_INFINIBAND_USER_MEM */
>
> #include <linux/err.h>
> @@ -230,5 +232,11 @@ static inline void ib_umem_dmabuf_revoke_lock(struct ib_umem_dmabuf *umem_dmabuf
> static inline void ib_umem_dmabuf_revoke_unlock(struct ib_umem_dmabuf *umem_dmabuf) {}
> static inline void ib_umem_dmabuf_revoke(struct ib_umem_dmabuf *umem_dmabuf) {}
>
> +static inline int ib_umem_check_rereg(struct ib_umem *umem, int flags,
> + int new_access_flags)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> #endif /* CONFIG_INFINIBAND_USER_MEM */
> #endif /* IB_UMEM_H */
>
> base-commit: 323c98a4ff06aa28114f2bf658fb43eb3b536bbc
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] RDMA: During rereg_mr ensure that REREG_ACCESS is compatible
2026-06-08 22:38 ` yanjun.zhu
@ 2026-06-08 23:25 ` Jason Gunthorpe
0 siblings, 0 replies; 3+ messages in thread
From: Jason Gunthorpe @ 2026-06-08 23:25 UTC (permalink / raw)
To: yanjun.zhu
Cc: Junxian Huang, Krzysztof Czurylo, linux-rdma, Chengchang Tang,
Tatyana Nikolova, Yishai Hadas, Zhu Yanjun, Andrew Morton,
David Hildenbrand, Leon Romanovsky, patches, Philip Tsukerman,
stable
On Mon, Jun 08, 2026 at 03:38:32PM -0700, yanjun.zhu wrote:
> But I found the following problem. I am not sure if we fix this problem in
> this commit or file a new commit.
The core code does all of that
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-08 23:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-08 16:44 [PATCH] RDMA: During rereg_mr ensure that REREG_ACCESS is compatible Jason Gunthorpe
2026-06-08 22:38 ` yanjun.zhu
2026-06-08 23:25 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox