Rather than using svc_get() and svc_put() to hold a stable reference to
the nfsd_svc for netlink lookups, simply hold the mutex for the entire
time.
The "entire" time isn't very long, and the mutex is not often contented.
This makes way for use to remove the refcounts of svc, which is more
confusing than useful.
Signed-off-by: NeilBrown <[email protected]>
---
fs/nfsd/nfsctl.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index d78ae4452946..8f644f1d157c 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1515,11 +1515,10 @@ int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb)
int ret = -ENODEV;
mutex_lock(&nfsd_mutex);
- if (nn->nfsd_serv) {
- svc_get(nn->nfsd_serv);
+ if (nn->nfsd_serv)
ret = 0;
- }
- mutex_unlock(&nfsd_mutex);
+ else
+ mutex_unlock(&nfsd_mutex);
return ret;
}
@@ -1691,8 +1690,6 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
*/
int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
{
- mutex_lock(&nfsd_mutex);
- nfsd_put(sock_net(cb->skb->sk));
mutex_unlock(&nfsd_mutex);
return 0;
--
2.42.0
On Mon, 2023-10-30 at 12:08 +1100, NeilBrown wrote:
> Rather than using svc_get() and svc_put() to hold a stable reference to
> the nfsd_svc for netlink lookups, simply hold the mutex for the entire
> time.
>
> The "entire" time isn't very long, and the mutex is not often contented.
>
> This makes way for use to remove the refcounts of svc, which is more
> confusing than useful.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> fs/nfsd/nfsctl.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index d78ae4452946..8f644f1d157c 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1515,11 +1515,10 @@ int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb)
> int ret = -ENODEV;
>
> mutex_lock(&nfsd_mutex);
> - if (nn->nfsd_serv) {
> - svc_get(nn->nfsd_serv);
> + if (nn->nfsd_serv)
> ret = 0;
> - }
> - mutex_unlock(&nfsd_mutex);
> + else
> + mutex_unlock(&nfsd_mutex);
>
> return ret;
> }
> @@ -1691,8 +1690,6 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
> */
> int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
> {
> - mutex_lock(&nfsd_mutex);
> - nfsd_put(sock_net(cb->skb->sk));
> mutex_unlock(&nfsd_mutex);
>
> return 0;
(cc'ing Lorenzo since he wrote this)
I think Lorenzo did it this way originally, and I convinced him to take
a reference instead. This should be fine though.
Reviewed-by: Jeff Layton <[email protected]>
> On Mon, 2023-10-30 at 12:08 +1100, NeilBrown wrote:
> > Rather than using svc_get() and svc_put() to hold a stable reference to
> > the nfsd_svc for netlink lookups, simply hold the mutex for the entire
> > time.
> >
> > The "entire" time isn't very long, and the mutex is not often contented.
> >
> > This makes way for use to remove the refcounts of svc, which is more
> > confusing than useful.
> >
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > fs/nfsd/nfsctl.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index d78ae4452946..8f644f1d157c 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -1515,11 +1515,10 @@ int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb)
> > int ret = -ENODEV;
> >
> > mutex_lock(&nfsd_mutex);
> > - if (nn->nfsd_serv) {
> > - svc_get(nn->nfsd_serv);
> > + if (nn->nfsd_serv)
> > ret = 0;
> > - }
> > - mutex_unlock(&nfsd_mutex);
> > + else
> > + mutex_unlock(&nfsd_mutex);
> >
> > return ret;
> > }
> > @@ -1691,8 +1690,6 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
> > */
> > int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
> > {
> > - mutex_lock(&nfsd_mutex);
> > - nfsd_put(sock_net(cb->skb->sk));
> > mutex_unlock(&nfsd_mutex);
> >
> > return 0;
>
> (cc'ing Lorenzo since he wrote this)
>
> I think Lorenzo did it this way originally, and I convinced him to take
> a reference instead. This should be fine though.
yep, the idea was to avoid grabbing the mutex and dump the refcount instead but
I think it is fine.
Regards,
Lorenzo
>
> Reviewed-by: Jeff Layton <[email protected]>