From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Xu Yilun <yilun.xu@linux.intel.com>
Cc: Marco Pagani <marpagan@redhat.com>,
Moritz Fischer <mdf@kernel.org>, Wu Hao <hao.wu@intel.com>,
Xu Yilun <yilun.xu@intel.com>, Tom Rix <trix@redhat.com>,
Alan Tull <atull@opensource.altera.com>,
linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] fpga: remove module reference counting from core components
Date: Wed, 8 Nov 2023 17:20:53 +0100 [thread overview]
Message-ID: <2023110839-jam-relapsing-7f5d@gregkh> (raw)
In-Reply-To: <ZUuu1CgVd4h3Qqu7@yilunxu-OptiPlex-7050>
On Wed, Nov 08, 2023 at 11:52:52PM +0800, Xu Yilun wrote:
> > >>
> > >> In fpga_region_get() / fpga_region_put(): call get_device() before
> > >> acquiring the mutex and put_device() after having released the mutex
> > >> to avoid races.
Why do you need another reference count with a lock? You already have
that with the calls to get/put_device().
> > > Could you help elaborate more about the race?
> > >
> >
> > I accidentally misused the word race. My concern was that memory might
> > be released after the last put_device(), causing mutex_unlock() to be
> > called on a mutex that does not exist anymore. It should not happen
> > for the moment since the region does not use devres, but I think it
> > still makes the code more brittle.
>
> It makes sense.
>
> But I dislike the mutex itself. The purpose is to exclusively grab the
> device, but a mutex is too much heavy for this.
Why "heavy"? Is this a fast-path? Have you measured it?
> The lock/unlock of mutex
> scattered in different functions is also an uncommon style. Maybe some
> atomic count should be enough.
Don't make a fake lock with an atomic variable, use real locks if you
need it.
Or don't even care about module unloading at all! Why does it matter?
It can never happen without explicit intervention and it's something
that a lot of the time, just will cause problems. Don't do a lot of
extra work for something that doesn't matter.
thanks,
greg k-h
next prev parent reply other threads:[~2023-11-08 16:22 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20231027152928.184012-1-marpagan@redhat.com>
2023-10-30 8:32 ` [RFC PATCH] fpga: remove module reference counting from core components Xu Yilun
2023-11-03 20:31 ` Marco Pagani
2023-11-08 15:52 ` Xu Yilun
2023-11-08 16:20 ` Greg Kroah-Hartman [this message]
2023-11-09 5:07 ` Xu Yilun
2023-11-09 5:27 ` Greg Kroah-Hartman
2023-11-09 7:16 ` Xu Yilun
2023-11-09 7:30 ` Greg Kroah-Hartman
2023-11-09 11:33 ` Marco Pagani
2023-11-10 22:58 ` Marco Pagani
2023-11-11 11:02 ` Greg Kroah-Hartman
2023-11-14 6:53 ` Xu Yilun
2023-11-17 21:58 ` Marco Pagani
2023-11-17 22:06 ` Greg Kroah-Hartman
2023-11-18 11:58 ` Xu Yilun
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=2023110839-jam-relapsing-7f5d@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=atull@opensource.altera.com \
--cc=hao.wu@intel.com \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marpagan@redhat.com \
--cc=mdf@kernel.org \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.com \
--cc=yilun.xu@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).