From: Leon Romanovsky <leon@kernel.org>
To: Zhu Yanjun <yanjun.zhu@linux.dev>
Cc: Li Zhijian <lizhijian@fujitsu.com>,
linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
zyjzyj2000@gmail.com, jgg@ziepe.ca,
Daisuke Matsuda <dskmtsd@gmail.com>
Subject: Re: [PATCH] IB/rxe: ODP: Fix missing umem_odp->umem_mutex unlock
Date: Tue, 30 Dec 2025 11:27:51 +0200 [thread overview]
Message-ID: <20251230092751.GA27868@unreal> (raw)
In-Reply-To: <deb756fc-dc58-4689-ac0e-632944830871@linux.dev>
On Fri, Dec 26, 2025 at 05:05:09PM -0800, Zhu Yanjun wrote:
>
> 在 2025/12/26 1:41, Li Zhijian 写道:
> > rxe_odp_map_range_and_lock() should unlock umem_odp->umem_mutex on error.
> >
> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> > ---
> > drivers/infiniband/sw/rxe/rxe_odp.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_odp.c b/drivers/infiniband/sw/rxe/rxe_odp.c
> > index 8b6a8b064d3c..d22b08da2713 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_odp.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_odp.c
> > @@ -178,8 +178,10 @@ static int rxe_odp_map_range_and_lock(struct rxe_mr *mr, u64 iova, int length, u
> > return err;
> 160 static int rxe_odp_map_range_and_lock(struct rxe_mr *mr, u64 iova, int
> length, u32 flags)
> 161 {
> 162 struct ib_umem_odp *umem_odp = to_ib_umem_odp(mr->umem);
> 163 bool need_fault;
> 164 int err;
> 165
> 166 if (unlikely(length < 1))
> 167 return -EINVAL;
> 168
> 169 mutex_lock(&umem_odp->umem_mutex);
> 170
> 171 need_fault = rxe_check_pagefault(umem_odp, iova, length);
> 172 if (need_fault) {
> 173 mutex_unlock(&umem_odp->umem_mutex);
> 174
> 175 /* umem_mutex is locked on success. */
> 176 err = rxe_odp_do_pagefault_and_lock(mr, iova, length,
> 177 flags);
> 178 if (err < 0)
>
> 179 return err;
>
> If the function rxe_odp_do_pagefault_and_lock() succeeds and run here, the
> mutex lock umem_mutex should be held.
>
> Thus, if rxe_check_pagefault fails, the mutex lock umem_mutex should be
> released.
>
> as such, I am fine with this.
>
> But IMO, the commit log is too simple. The above explanations should be
> added into the commit logs.
>
> Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>
I made a few adjustments to the commit message, added a Fixes tag, included
your Reviewed-by, and applied the patch.
Thanks
>
> Zhu Yanjun
>
> 180
> 181 need_fault = rxe_check_pagefault(umem_odp, iova, length);
> 182 if (need_fault)
> 183 return -EFAULT;
> 184 }
> 185
> 186 return 0;
> 187 }
>
> > need_fault = rxe_check_pagefault(umem_odp, iova, length);
> > - if (need_fault)
> > + if (need_fault) {
> > + mutex_unlock(&umem_odp->umem_mutex);
> > return -EFAULT;
> > + }
> > }
> > return 0;
>
> --
> Best Regards,
> Yanjun.Zhu
>
next prev parent reply other threads:[~2025-12-30 9:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-26 9:41 [PATCH] IB/rxe: ODP: Fix missing umem_odp->umem_mutex unlock Li Zhijian
2025-12-27 1:05 ` Zhu Yanjun
2025-12-30 9:27 ` Leon Romanovsky [this message]
2025-12-30 15:24 ` Zhu Yanjun
2025-12-30 9:28 ` Leon Romanovsky
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=20251230092751.GA27868@unreal \
--to=leon@kernel.org \
--cc=dskmtsd@gmail.com \
--cc=jgg@ziepe.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=lizhijian@fujitsu.com \
--cc=yanjun.zhu@linux.dev \
--cc=zyjzyj2000@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 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.