diff mbox

fs-pin: allow pin_remove() to be called other than from ->kill()

Message ID 20150729135914.13cb0f86@noble (mailing list archive)
State New, archived
Headers show

Commit Message

NeilBrown July 29, 2015, 3:59 a.m. UTC
fs-pin currently assumes when either the vfsmount or the fs_pin wants
to unpin, pin_kill() will be called.
This requires that the ->kill() function can wait for any transient
references to the fs_pin to be released.  If the structure containing
the fs_pin doesn't already have the ability to wait for references,
this can be a burden.

As the fs_pin already has infrastructure for waiting, that can be
leveraged to remove the burden.

In this alternate scenario, only the vfsmount calls pin_kill() when it
wants to unpin.  The owner of the fs_pin() instead calls pin_remove().

The ->kill() function removes any long-term references, and then calls
pin_kill() (recursively).
When the last reference on (the structure containing) the fs_pin is
dropped, pin_remove() will be called and the (recursive) pin_kill()
call will complete.

For this to be safe, the final "put" must *not* free the structure if
pin_kill() has already been called, as that could leave ->kill()
accessing freed data.

So we provide a return value for pin_remove() which reports the old
->done value.

When final put calls pin_remove() it checks that value.
If it was 0, then pin_kill() has not called ->kill and will not,
so final put can free the data structure.
If it was -1, then pin_kill() has called ->kill, and ->kill will
free the data structure - final put must not touch it.

This makes the 'wait' infrastructure of fs_pin available to any
pinning client which wants to use it.

Signed-Off-By: NeilBrown <neilb@suse.com>

---
Hi Al,
 do you see this as a workable solution?  I think it will improve the nfsd pining patch
a lot.

Thanks,
NeilBrown


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Kinglong Mee Aug. 10, 2015, 11:37 a.m. UTC | #1
Ping ...

Hello Viro,
What's your opinion at this patch ?

If Okay, I will update those patches based on this one.

thanks,
Kinglong Mee

On 7/29/2015 11:59, NeilBrown wrote:
> 
> 
> fs-pin currently assumes when either the vfsmount or the fs_pin wants
> to unpin, pin_kill() will be called.
> This requires that the ->kill() function can wait for any transient
> references to the fs_pin to be released.  If the structure containing
> the fs_pin doesn't already have the ability to wait for references,
> this can be a burden.
> 
> As the fs_pin already has infrastructure for waiting, that can be
> leveraged to remove the burden.
> 
> In this alternate scenario, only the vfsmount calls pin_kill() when it
> wants to unpin.  The owner of the fs_pin() instead calls pin_remove().
> 
> The ->kill() function removes any long-term references, and then calls
> pin_kill() (recursively).
> When the last reference on (the structure containing) the fs_pin is
> dropped, pin_remove() will be called and the (recursive) pin_kill()
> call will complete.
> 
> For this to be safe, the final "put" must *not* free the structure if
> pin_kill() has already been called, as that could leave ->kill()
> accessing freed data.
> 
> So we provide a return value for pin_remove() which reports the old
> ->done value.
> 
> When final put calls pin_remove() it checks that value.
> If it was 0, then pin_kill() has not called ->kill and will not,
> so final put can free the data structure.
> If it was -1, then pin_kill() has called ->kill, and ->kill will
> free the data structure - final put must not touch it.
> 
> This makes the 'wait' infrastructure of fs_pin available to any
> pinning client which wants to use it.
> 
> Signed-Off-By: NeilBrown <neilb@suse.com>
> 
> ---
> Hi Al,
>  do you see this as a workable solution?  I think it will improve the nfsd pining patch
> a lot.
> 
> Thanks,
> NeilBrown
> 
> 
> diff --git a/fs/fs_pin.c b/fs/fs_pin.c
> index 611b5408f6ec..b7954a9d17da 100644
> --- a/fs/fs_pin.c
> +++ b/fs/fs_pin.c
> @@ -6,16 +6,32 @@
>  
>  static DEFINE_SPINLOCK(pin_lock);
>  
> -void pin_remove(struct fs_pin *pin)
> +/**
> + * pin_remove - disconnect an fs_pin from the pinned structure.
> + * @pin:	The struct fs_pin which is pinning something.
> + *
> + * Detach a 'pin' which was added by pin_insert().  A return value
> + * of -1 implies that pin_kill() has already been called and that the
> + * ->kill() function now owns the data structure containing @pin.
> + * The function which called pin_remove() must not touch the data structure
> + * again (unless it is the ->kill() function itself).
> + * A return value of 0 implies an uneventful disconnect: pin_kill() has not called,
> + * and will not call, the ->kill() function on this @pin.
> + * Any other return value is a usage error - e.g. repeated call to pin_remove().
> + */
> +int pin_remove(struct fs_pin *pin)
>  {
> +	int ret;
>  	spin_lock(&pin_lock);
>  	hlist_del_init(&pin->m_list);
>  	hlist_del_init(&pin->s_list);
>  	spin_unlock(&pin_lock);
>  	spin_lock_irq(&pin->wait.lock);
> +	ret = pin->done;
>  	pin->done = 1;
>  	wake_up_locked(&pin->wait);
>  	spin_unlock_irq(&pin->wait.lock);
> +	return ret;
>  }
>  
>  void pin_insert_group(struct fs_pin *pin, struct vfsmount *m, struct hlist_head *p)
> diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h
> index 3886b3bffd7f..2fe9d3ba09e8 100644
> --- a/include/linux/fs_pin.h
> +++ b/include/linux/fs_pin.h
> @@ -18,7 +18,7 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *))
>  	p->kill = kill;
>  }
>  
> -void pin_remove(struct fs_pin *);
> +int pin_remove(struct fs_pin *);
>  void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *);
>  void pin_insert(struct fs_pin *, struct vfsmount *);
>  void pin_kill(struct fs_pin *);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kinglong Mee Aug. 18, 2015, 6:07 a.m. UTC | #2
Sorry for my so late reply.

On 7/29/2015 11:59, NeilBrown wrote:
> fs-pin currently assumes when either the vfsmount or the fs_pin wants
> to unpin, pin_kill() will be called.
> This requires that the ->kill() function can wait for any transient
> references to the fs_pin to be released.  If the structure containing
> the fs_pin doesn't already have the ability to wait for references,
> this can be a burden.
> 
> As the fs_pin already has infrastructure for waiting, that can be
> leveraged to remove the burden.
> 
> In this alternate scenario, only the vfsmount calls pin_kill() when it
> wants to unpin.  The owner of the fs_pin() instead calls pin_remove().
> 
> The ->kill() function removes any long-term references, and then calls
> pin_kill() (recursively).
> When the last reference on (the structure containing) the fs_pin is
> dropped, pin_remove() will be called and the (recursive) pin_kill()
> call will complete.
> 
> For this to be safe, the final "put" must *not* free the structure if
> pin_kill() has already been called, as that could leave ->kill()
> accessing freed data.
> 
> So we provide a return value for pin_remove() which reports the old
> ->done value.
> 
> When final put calls pin_remove() it checks that value.
> If it was 0, then pin_kill() has not called ->kill and will not,
> so final put can free the data structure.
> If it was -1, then pin_kill() has called ->kill, and ->kill will
> free the data structure - final put must not touch it.

I find another problem, 
how can xxx_pin_kill known the last reference of the data have be put?

eg,
static void expkey_pin_kill(struct fs_pin *pin)
{
        struct svc_expkey *key = container_of(pin, struct svc_expkey, ek_pin);
        cache_delete_entry(key->cd, &key->h);
        expkey_destroy(key);
}

expkey_pin_kill has call cache_delete_entry() but doesn't know whether
the last reference has be put (here expkey_put is called)? 

Before the cache_list is deleted from the cache, a third user gets
the reference, so that, the third user will be the last put of the cache
by calling expkey_put, xxx_pin_kill only decrease the reference.

thanks,
Kinglong Mee

> 
> This makes the 'wait' infrastructure of fs_pin available to any
> pinning client which wants to use it.
> 
> Signed-Off-By: NeilBrown <neilb@suse.com>
> 
> ---
> Hi Al,
>  do you see this as a workable solution?  I think it will improve the nfsd pining patch
> a lot.
> 
> Thanks,
> NeilBrown
> 
> 
> diff --git a/fs/fs_pin.c b/fs/fs_pin.c
> index 611b5408f6ec..b7954a9d17da 100644
> --- a/fs/fs_pin.c
> +++ b/fs/fs_pin.c
> @@ -6,16 +6,32 @@
>  
>  static DEFINE_SPINLOCK(pin_lock);
>  
> -void pin_remove(struct fs_pin *pin)
> +/**
> + * pin_remove - disconnect an fs_pin from the pinned structure.
> + * @pin:	The struct fs_pin which is pinning something.
> + *
> + * Detach a 'pin' which was added by pin_insert().  A return value
> + * of -1 implies that pin_kill() has already been called and that the
> + * ->kill() function now owns the data structure containing @pin.
> + * The function which called pin_remove() must not touch the data structure
> + * again (unless it is the ->kill() function itself).
> + * A return value of 0 implies an uneventful disconnect: pin_kill() has not called,
> + * and will not call, the ->kill() function on this @pin.
> + * Any other return value is a usage error - e.g. repeated call to pin_remove().
> + */
> +int pin_remove(struct fs_pin *pin)
>  {
> +	int ret;
>  	spin_lock(&pin_lock);
>  	hlist_del_init(&pin->m_list);
>  	hlist_del_init(&pin->s_list);
>  	spin_unlock(&pin_lock);
>  	spin_lock_irq(&pin->wait.lock);
> +	ret = pin->done;
>  	pin->done = 1;
>  	wake_up_locked(&pin->wait);
>  	spin_unlock_irq(&pin->wait.lock);
> +	return ret;
>  }
>  
>  void pin_insert_group(struct fs_pin *pin, struct vfsmount *m, struct hlist_head *p)
> diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h
> index 3886b3bffd7f..2fe9d3ba09e8 100644
> --- a/include/linux/fs_pin.h
> +++ b/include/linux/fs_pin.h
> @@ -18,7 +18,7 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *))
>  	p->kill = kill;
>  }
>  
> -void pin_remove(struct fs_pin *);
> +int pin_remove(struct fs_pin *);
>  void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *);
>  void pin_insert(struct fs_pin *, struct vfsmount *);
>  void pin_kill(struct fs_pin *);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
NeilBrown Aug. 18, 2015, 6:21 a.m. UTC | #3
On Tue, 18 Aug 2015 14:07:58 +0800 Kinglong Mee <kinglongmee@gmail.com>
wrote:

> Sorry for my so late reply.
> 
> On 7/29/2015 11:59, NeilBrown wrote:
> > fs-pin currently assumes when either the vfsmount or the fs_pin wants
> > to unpin, pin_kill() will be called.
> > This requires that the ->kill() function can wait for any transient
> > references to the fs_pin to be released.  If the structure containing
> > the fs_pin doesn't already have the ability to wait for references,
> > this can be a burden.
> > 
> > As the fs_pin already has infrastructure for waiting, that can be
> > leveraged to remove the burden.
> > 
> > In this alternate scenario, only the vfsmount calls pin_kill() when it
> > wants to unpin.  The owner of the fs_pin() instead calls pin_remove().
> > 
> > The ->kill() function removes any long-term references, and then calls
> > pin_kill() (recursively).
> > When the last reference on (the structure containing) the fs_pin is
> > dropped, pin_remove() will be called and the (recursive) pin_kill()
> > call will complete.
> > 
> > For this to be safe, the final "put" must *not* free the structure if
> > pin_kill() has already been called, as that could leave ->kill()
> > accessing freed data.
> > 
> > So we provide a return value for pin_remove() which reports the old
> > ->done value.
> > 
> > When final put calls pin_remove() it checks that value.
> > If it was 0, then pin_kill() has not called ->kill and will not,
> > so final put can free the data structure.
> > If it was -1, then pin_kill() has called ->kill, and ->kill will
> > free the data structure - final put must not touch it.
> 
> I find another problem, 
> how can xxx_pin_kill known the last reference of the data have be put?
> 
> eg,
> static void expkey_pin_kill(struct fs_pin *pin)
> {
>         struct svc_expkey *key = container_of(pin, struct svc_expkey, ek_pin);
>         cache_delete_entry(key->cd, &key->h);
>         expkey_destroy(key);
> }
> 
> expkey_pin_kill has call cache_delete_entry() but doesn't know whether
> the last reference has be put (here expkey_put is called)? 
> 
> Before the cache_list is deleted from the cache, a third user gets
> the reference, so that, the third user will be the last put of the cache
> by calling expkey_put, xxx_pin_kill only decrease the reference.

expkey_pin_kill() should call:
  cache_delete_entry()
  pin_kill()
  expkey_destroy()

The "cache_delete_entry()" call removes the only long-term reference.
Any other reference will be transient so it is safe to wait for those.

The 'pin_kill()' call will wait of pin_remove() to be called (it
already does that).
pin_remove() will be called when the last reference is dropped.  As
described above, that pin_remove call will return -1 and so the 'put'
function will not have called expkey_destroy.

Finally the expkey_destroy() function actually frees the data
structure.  No other code can be touching at this point.

Thanks,
NeilBrown


> 
> thanks,
> Kinglong Mee
> 
> > 
> > This makes the 'wait' infrastructure of fs_pin available to any
> > pinning client which wants to use it.
> > 
> > Signed-Off-By: NeilBrown <neilb@suse.com>
> > 
> > ---
> > Hi Al,
> >  do you see this as a workable solution?  I think it will improve the nfsd pining patch
> > a lot.
> > 
> > Thanks,
> > NeilBrown
> > 
> > 
> > diff --git a/fs/fs_pin.c b/fs/fs_pin.c
> > index 611b5408f6ec..b7954a9d17da 100644
> > --- a/fs/fs_pin.c
> > +++ b/fs/fs_pin.c
> > @@ -6,16 +6,32 @@
> >  
> >  static DEFINE_SPINLOCK(pin_lock);
> >  
> > -void pin_remove(struct fs_pin *pin)
> > +/**
> > + * pin_remove - disconnect an fs_pin from the pinned structure.
> > + * @pin:	The struct fs_pin which is pinning something.
> > + *
> > + * Detach a 'pin' which was added by pin_insert().  A return value
> > + * of -1 implies that pin_kill() has already been called and that the
> > + * ->kill() function now owns the data structure containing @pin.
> > + * The function which called pin_remove() must not touch the data structure
> > + * again (unless it is the ->kill() function itself).
> > + * A return value of 0 implies an uneventful disconnect: pin_kill() has not called,
> > + * and will not call, the ->kill() function on this @pin.
> > + * Any other return value is a usage error - e.g. repeated call to pin_remove().
> > + */
> > +int pin_remove(struct fs_pin *pin)
> >  {
> > +	int ret;
> >  	spin_lock(&pin_lock);
> >  	hlist_del_init(&pin->m_list);
> >  	hlist_del_init(&pin->s_list);
> >  	spin_unlock(&pin_lock);
> >  	spin_lock_irq(&pin->wait.lock);
> > +	ret = pin->done;
> >  	pin->done = 1;
> >  	wake_up_locked(&pin->wait);
> >  	spin_unlock_irq(&pin->wait.lock);
> > +	return ret;
> >  }
> >  
> >  void pin_insert_group(struct fs_pin *pin, struct vfsmount *m, struct hlist_head *p)
> > diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h
> > index 3886b3bffd7f..2fe9d3ba09e8 100644
> > --- a/include/linux/fs_pin.h
> > +++ b/include/linux/fs_pin.h
> > @@ -18,7 +18,7 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *))
> >  	p->kill = kill;
> >  }
> >  
> > -void pin_remove(struct fs_pin *);
> > +int pin_remove(struct fs_pin *);
> >  void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *);
> >  void pin_insert(struct fs_pin *, struct vfsmount *);
> >  void pin_kill(struct fs_pin *);
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kinglong Mee Aug. 18, 2015, 6:37 a.m. UTC | #4
On 8/18/2015 14:21, NeilBrown wrote:
> On Tue, 18 Aug 2015 14:07:58 +0800 Kinglong Mee <kinglongmee@gmail.com>
> wrote:
> 
>> Sorry for my so late reply.
>>
>> On 7/29/2015 11:59, NeilBrown wrote:
>>> fs-pin currently assumes when either the vfsmount or the fs_pin wants
>>> to unpin, pin_kill() will be called.
>>> This requires that the ->kill() function can wait for any transient
>>> references to the fs_pin to be released.  If the structure containing
>>> the fs_pin doesn't already have the ability to wait for references,
>>> this can be a burden.
>>>
>>> As the fs_pin already has infrastructure for waiting, that can be
>>> leveraged to remove the burden.
>>>
>>> In this alternate scenario, only the vfsmount calls pin_kill() when it
>>> wants to unpin.  The owner of the fs_pin() instead calls pin_remove().
>>>
>>> The ->kill() function removes any long-term references, and then calls
>>> pin_kill() (recursively).
>>> When the last reference on (the structure containing) the fs_pin is
>>> dropped, pin_remove() will be called and the (recursive) pin_kill()
>>> call will complete.
>>>
>>> For this to be safe, the final "put" must *not* free the structure if
>>> pin_kill() has already been called, as that could leave ->kill()
>>> accessing freed data.
>>>
>>> So we provide a return value for pin_remove() which reports the old
>>> ->done value.
>>>
>>> When final put calls pin_remove() it checks that value.
>>> If it was 0, then pin_kill() has not called ->kill and will not,
>>> so final put can free the data structure.
>>> If it was -1, then pin_kill() has called ->kill, and ->kill will
>>> free the data structure - final put must not touch it.
>>
>> I find another problem, 
>> how can xxx_pin_kill known the last reference of the data have be put?
>>
>> eg,
>> static void expkey_pin_kill(struct fs_pin *pin)
>> {
>>         struct svc_expkey *key = container_of(pin, struct svc_expkey, ek_pin);
>>         cache_delete_entry(key->cd, &key->h);
>>         expkey_destroy(key);
>> }
>>
>> expkey_pin_kill has call cache_delete_entry() but doesn't know whether
>> the last reference has be put (here expkey_put is called)? 
>>
>> Before the cache_list is deleted from the cache, a third user gets
>> the reference, so that, the third user will be the last put of the cache
>> by calling expkey_put, xxx_pin_kill only decrease the reference.
> 
> expkey_pin_kill() should call:
>   cache_delete_entry()
>   pin_kill()
>   expkey_destroy()
> 
> The "cache_delete_entry()" call removes the only long-term reference.
> Any other reference will be transient so it is safe to wait for those.
> 
> The 'pin_kill()' call will wait of pin_remove() to be called (it
> already does that).

Sorry for my missing of calling pin_kill() here.

> pin_remove() will be called when the last reference is dropped.  As
> described above, that pin_remove call will return -1 and so the 'put'
> function will not have called expkey_destroy.
> 
> Finally the expkey_destroy() function actually frees the data
> structure.  No other code can be touching at this point.

With calling pin_kill() again in expkey_pin_kill makes every clear now.
Thanks again.

The only thing is waiting Al's opinion. 

thanks,
Kinglong Mee

> 
> Thanks,
> NeilBrown
> 
> 
>>
>> thanks,
>> Kinglong Mee
>>
>>>
>>> This makes the 'wait' infrastructure of fs_pin available to any
>>> pinning client which wants to use it.
>>>
>>> Signed-Off-By: NeilBrown <neilb@suse.com>
>>>
>>> ---
>>> Hi Al,
>>>  do you see this as a workable solution?  I think it will improve the nfsd pining patch
>>> a lot.
>>>
>>> Thanks,
>>> NeilBrown
>>>
>>>
>>> diff --git a/fs/fs_pin.c b/fs/fs_pin.c
>>> index 611b5408f6ec..b7954a9d17da 100644
>>> --- a/fs/fs_pin.c
>>> +++ b/fs/fs_pin.c
>>> @@ -6,16 +6,32 @@
>>>  
>>>  static DEFINE_SPINLOCK(pin_lock);
>>>  
>>> -void pin_remove(struct fs_pin *pin)
>>> +/**
>>> + * pin_remove - disconnect an fs_pin from the pinned structure.
>>> + * @pin:	The struct fs_pin which is pinning something.
>>> + *
>>> + * Detach a 'pin' which was added by pin_insert().  A return value
>>> + * of -1 implies that pin_kill() has already been called and that the
>>> + * ->kill() function now owns the data structure containing @pin.
>>> + * The function which called pin_remove() must not touch the data structure
>>> + * again (unless it is the ->kill() function itself).
>>> + * A return value of 0 implies an uneventful disconnect: pin_kill() has not called,
>>> + * and will not call, the ->kill() function on this @pin.
>>> + * Any other return value is a usage error - e.g. repeated call to pin_remove().
>>> + */
>>> +int pin_remove(struct fs_pin *pin)
>>>  {
>>> +	int ret;
>>>  	spin_lock(&pin_lock);
>>>  	hlist_del_init(&pin->m_list);
>>>  	hlist_del_init(&pin->s_list);
>>>  	spin_unlock(&pin_lock);
>>>  	spin_lock_irq(&pin->wait.lock);
>>> +	ret = pin->done;
>>>  	pin->done = 1;
>>>  	wake_up_locked(&pin->wait);
>>>  	spin_unlock_irq(&pin->wait.lock);
>>> +	return ret;
>>>  }
>>>  
>>>  void pin_insert_group(struct fs_pin *pin, struct vfsmount *m, struct hlist_head *p)
>>> diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h
>>> index 3886b3bffd7f..2fe9d3ba09e8 100644
>>> --- a/include/linux/fs_pin.h
>>> +++ b/include/linux/fs_pin.h
>>> @@ -18,7 +18,7 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *))
>>>  	p->kill = kill;
>>>  }
>>>  
>>> -void pin_remove(struct fs_pin *);
>>> +int pin_remove(struct fs_pin *);
>>>  void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *);
>>>  void pin_insert(struct fs_pin *, struct vfsmount *);
>>>  void pin_kill(struct fs_pin *);
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/fs_pin.c b/fs/fs_pin.c
index 611b5408f6ec..b7954a9d17da 100644
--- a/fs/fs_pin.c
+++ b/fs/fs_pin.c
@@ -6,16 +6,32 @@ 
 
 static DEFINE_SPINLOCK(pin_lock);
 
-void pin_remove(struct fs_pin *pin)
+/**
+ * pin_remove - disconnect an fs_pin from the pinned structure.
+ * @pin:	The struct fs_pin which is pinning something.
+ *
+ * Detach a 'pin' which was added by pin_insert().  A return value
+ * of -1 implies that pin_kill() has already been called and that the
+ * ->kill() function now owns the data structure containing @pin.
+ * The function which called pin_remove() must not touch the data structure
+ * again (unless it is the ->kill() function itself).
+ * A return value of 0 implies an uneventful disconnect: pin_kill() has not called,
+ * and will not call, the ->kill() function on this @pin.
+ * Any other return value is a usage error - e.g. repeated call to pin_remove().
+ */
+int pin_remove(struct fs_pin *pin)
 {
+	int ret;
 	spin_lock(&pin_lock);
 	hlist_del_init(&pin->m_list);
 	hlist_del_init(&pin->s_list);
 	spin_unlock(&pin_lock);
 	spin_lock_irq(&pin->wait.lock);
+	ret = pin->done;
 	pin->done = 1;
 	wake_up_locked(&pin->wait);
 	spin_unlock_irq(&pin->wait.lock);
+	return ret;
 }
 
 void pin_insert_group(struct fs_pin *pin, struct vfsmount *m, struct hlist_head *p)
diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h
index 3886b3bffd7f..2fe9d3ba09e8 100644
--- a/include/linux/fs_pin.h
+++ b/include/linux/fs_pin.h
@@ -18,7 +18,7 @@  static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *))
 	p->kill = kill;
 }
 
-void pin_remove(struct fs_pin *);
+int pin_remove(struct fs_pin *);
 void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *);
 void pin_insert(struct fs_pin *, struct vfsmount *);
 void pin_kill(struct fs_pin *);