diff mbox

[v3,03/10] NFSD: Clean up forgetting clients

Message ID 1354026919-13313-4-git-send-email-bjschuma@netapp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bryan Schumaker Nov. 27, 2012, 2:35 p.m. UTC
From: Bryan Schumaker <bjschuma@netapp.com>

I added in a generic for-each loop that takes a pass over the client_lru
list and calls some function.  The next few patches will update other
operations to use this function as well.  A value of 0 still means "forget
everything that is found".

Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
---
 fs/nfsd/nfs4state.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

J. Bruce Fields Nov. 28, 2012, 4:29 p.m. UTC | #1
On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
> From: Bryan Schumaker <bjschuma@netapp.com>
> 
> I added in a generic for-each loop that takes a pass over the client_lru
> list and calls some function.  The next few patches will update other
> operations to use this function as well.  A value of 0 still means "forget
> everything that is found".
> 
> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> ---
>  fs/nfsd/nfs4state.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 050a35e..07abca5 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4591,19 +4591,32 @@ nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct nfsd_net *nn)
>  
>  #ifdef CONFIG_NFSD_FAULT_INJECTION
>  
> -void nfsd_forget_clients(u64 num)
> +u64 nfsd_forget_client(struct nfsd_net *nn, struct nfs4_client *clp, u64 max)

It doesn't look like you really need nfsd_net?

--b.

> +{
> +	nfsd4_client_record_remove(clp);
> +	expire_client(clp);
> +	return 1;
> +}
> +
> +static u64 nfsd_for_n_state(u64 max, u64 (*func)(struct nfsd_net *, struct nfs4_client *, u64))
>  {
>  	struct nfs4_client *clp, *next;
> -	int count = 0;
> +	u64 count = 0;
>  	struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id);
>  
>  	list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) {
> -		expire_client(clp);
> -		if (++count == num)
> +		count += func(nn, clp, max - count);
> +		if ((max != 0) && (count >= max))
>  			break;
>  	}
>  
> -	printk(KERN_INFO "NFSD: Forgot %d clients", count);
> +	return count;
> +}
> +
> +void nfsd_forget_clients(u64 num)
> +{
> +	u64 count = nfsd_for_n_state(num, nfsd_forget_client);
> +	printk(KERN_INFO "NFSD: Forgot %llu clients", count);
>  }
>  
>  static void release_lockowner_sop(struct nfs4_stateowner *sop)
> -- 
> 1.8.0.1
> 
--
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
Bryan Schumaker Nov. 28, 2012, 4:34 p.m. UTC | #2
On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
> On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
>> From: Bryan Schumaker <bjschuma@netapp.com>
>>
>> I added in a generic for-each loop that takes a pass over the client_lru
>> list and calls some function.  The next few patches will update other
>> operations to use this function as well.  A value of 0 still means "forget
>> everything that is found".
>>
>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
>> ---
>>  fs/nfsd/nfs4state.c | 23 ++++++++++++++++++-----
>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 050a35e..07abca5 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -4591,19 +4591,32 @@ nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct nfsd_net *nn)
>>  
>>  #ifdef CONFIG_NFSD_FAULT_INJECTION
>>  
>> -void nfsd_forget_clients(u64 num)
>> +u64 nfsd_forget_client(struct nfsd_net *nn, struct nfs4_client *clp, u64 max)
> 
> It doesn't look like you really need nfsd_net?

Not for this, but I find locks through the nn->ownerstr_hashtable since the nfs4_client doesn't have a "cl_locks" list similar to "cl_openowners" (unless there is something I'm missing...?).  I figured I would introduce the function with the nfsd_net pointer early, rather than change it in the next patch.

I suppose I could have also put the locks patch first...

- Bryan

> 
> --b.
> 
>> +{
>> +	nfsd4_client_record_remove(clp);
>> +	expire_client(clp);
>> +	return 1;
>> +}
>> +
>> +static u64 nfsd_for_n_state(u64 max, u64 (*func)(struct nfsd_net *, struct nfs4_client *, u64))
>>  {
>>  	struct nfs4_client *clp, *next;
>> -	int count = 0;
>> +	u64 count = 0;
>>  	struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id);
>>  
>>  	list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) {
>> -		expire_client(clp);
>> -		if (++count == num)
>> +		count += func(nn, clp, max - count);
>> +		if ((max != 0) && (count >= max))
>>  			break;
>>  	}
>>  
>> -	printk(KERN_INFO "NFSD: Forgot %d clients", count);
>> +	return count;
>> +}
>> +
>> +void nfsd_forget_clients(u64 num)
>> +{
>> +	u64 count = nfsd_for_n_state(num, nfsd_forget_client);
>> +	printk(KERN_INFO "NFSD: Forgot %llu clients", count);
>>  }
>>  
>>  static void release_lockowner_sop(struct nfs4_stateowner *sop)
>> -- 
>> 1.8.0.1
>>

--
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
J. Bruce Fields Nov. 28, 2012, 4:47 p.m. UTC | #3
On Wed, Nov 28, 2012 at 11:34:40AM -0500, Bryan Schumaker wrote:
> On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
> > On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
> >> From: Bryan Schumaker <bjschuma@netapp.com>
> >>
> >> I added in a generic for-each loop that takes a pass over the
> >> client_lru list and calls some function.  The next few patches will
> >> update other operations to use this function as well.  A value of 0
> >> still means "forget everything that is found".
> >>
> >> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com> ---
> >> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++----- 1 file changed, 18
> >> insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
> >> 050a35e..07abca5 100644 --- a/fs/nfsd/nfs4state.c +++
> >> b/fs/nfsd/nfs4state.c @@ -4591,19 +4591,32 @@
> >> nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct
> >> nfsd_net *nn)
> >>  
> >>  #ifdef CONFIG_NFSD_FAULT_INJECTION
> >>  
> >> -void nfsd_forget_clients(u64 num) +u64 nfsd_forget_client(struct
> >> nfsd_net *nn, struct nfs4_client *clp, u64 max)
> > 
> > It doesn't look like you really need nfsd_net?
> 
> Not for this, but I find locks through the nn->ownerstr_hashtable
> since the nfs4_client doesn't have a "cl_locks" list similar to
> "cl_openowners" (unless there is something I'm missing...?).

That's because lockowners are all reachable from openowners (e.g. see
how it's done in
release_openowner->unhash_openowner->release_open_stateid->unhash_open_stateid->release_stateid_lockowners.
Yeah, it's a little tangled.).

Alternatively you can get to the network namespace from the client
(clp->net).

--b.

> I
> figured I would introduce the function with the nfsd_net pointer
> early, rather than change it in the next patch.
> 
> I suppose I could have also put the locks patch first...
> 
> - Bryan
> 
> > 
> > --b.
> > 
> >> +{ +	nfsd4_client_record_remove(clp); +
> >> expire_client(clp); +	return 1; +} + +static u64
> >> nfsd_for_n_state(u64 max, u64 (*func)(struct nfsd_net *, struct
> >> nfs4_client *, u64)) { struct nfs4_client *clp, *next; -	int
> >> count = 0; +	u64 count = 0; struct nfsd_net *nn =
> >> net_generic(current->nsproxy->net_ns, nfsd_net_id);
> >>  
> >>  	list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) { -
> >>  	expire_client(clp); -		if (++count == num) +
> >>  	count += func(nn, clp, max - count); +		if ((max != 0)
> >>  	&& (count >= max)) break; }
> >>  
> >> -	printk(KERN_INFO "NFSD: Forgot %d clients", count); +	return
> >> count; +} + +void nfsd_forget_clients(u64 num) +{ +	u64
> >> count = nfsd_for_n_state(num, nfsd_forget_client); +
> >> printk(KERN_INFO "NFSD: Forgot %llu clients", count); }
> >>  
> >>  static void release_lockowner_sop(struct nfs4_stateowner *sop) --
> >>  1.8.0.1
> >>
> 
> -- 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
--
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
Bryan Schumaker Nov. 28, 2012, 4:49 p.m. UTC | #4
On 11/28/2012 11:47 AM, J. Bruce Fields wrote:
> On Wed, Nov 28, 2012 at 11:34:40AM -0500, Bryan Schumaker wrote:
>> On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
>>> On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
>>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>>>
>>>> I added in a generic for-each loop that takes a pass over the
>>>> client_lru list and calls some function.  The next few patches will
>>>> update other operations to use this function as well.  A value of 0
>>>> still means "forget everything that is found".
>>>>
>>>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com> ---
>>>> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++----- 1 file changed, 18
>>>> insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
>>>> 050a35e..07abca5 100644 --- a/fs/nfsd/nfs4state.c +++
>>>> b/fs/nfsd/nfs4state.c @@ -4591,19 +4591,32 @@
>>>> nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct
>>>> nfsd_net *nn)
>>>>  
>>>>  #ifdef CONFIG_NFSD_FAULT_INJECTION
>>>>  
>>>> -void nfsd_forget_clients(u64 num) +u64 nfsd_forget_client(struct
>>>> nfsd_net *nn, struct nfs4_client *clp, u64 max)
>>>
>>> It doesn't look like you really need nfsd_net?
>>
>> Not for this, but I find locks through the nn->ownerstr_hashtable
>> since the nfs4_client doesn't have a "cl_locks" list similar to
>> "cl_openowners" (unless there is something I'm missing...?).
> 
> That's because lockowners are all reachable from openowners (e.g. see
> how it's done in
> release_openowner->unhash_openowner->release_open_stateid->unhash_open_stateid->release_stateid_lockowners.
> Yeah, it's a little tangled.).
> 
> Alternatively you can get to the network namespace from the client
> (clp->net).

Both are good to know!  I'll look at getting lockowners from openowners first.

- Bryan

> 
> --b.
> 
>> I
>> figured I would introduce the function with the nfsd_net pointer
>> early, rather than change it in the next patch.
>>
>> I suppose I could have also put the locks patch first...
>>
>> - Bryan
>>
>>>
>>> --b.
>>>
>>>> +{ +	nfsd4_client_record_remove(clp); +
>>>> expire_client(clp); +	return 1; +} + +static u64
>>>> nfsd_for_n_state(u64 max, u64 (*func)(struct nfsd_net *, struct
>>>> nfs4_client *, u64)) { struct nfs4_client *clp, *next; -	int
>>>> count = 0; +	u64 count = 0; struct nfsd_net *nn =
>>>> net_generic(current->nsproxy->net_ns, nfsd_net_id);
>>>>  
>>>>  	list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) { -
>>>>  	expire_client(clp); -		if (++count == num) +
>>>>  	count += func(nn, clp, max - count); +		if ((max != 0)
>>>>  	&& (count >= max)) break; }
>>>>  
>>>> -	printk(KERN_INFO "NFSD: Forgot %d clients", count); +	return
>>>> count; +} + +void nfsd_forget_clients(u64 num) +{ +	u64
>>>> count = nfsd_for_n_state(num, nfsd_forget_client); +
>>>> printk(KERN_INFO "NFSD: Forgot %llu clients", count); }
>>>>  
>>>>  static void release_lockowner_sop(struct nfs4_stateowner *sop) --
>>>>  1.8.0.1
>>>>
>>
>> -- 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

--
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
J. Bruce Fields Nov. 28, 2012, 4:56 p.m. UTC | #5
On Wed, Nov 28, 2012 at 11:49:53AM -0500, Bryan Schumaker wrote:
> On 11/28/2012 11:47 AM, J. Bruce Fields wrote:
> > On Wed, Nov 28, 2012 at 11:34:40AM -0500, Bryan Schumaker wrote:
> >> On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
> >>> On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
> >>>> From: Bryan Schumaker <bjschuma@netapp.com>
> >>>>
> >>>> I added in a generic for-each loop that takes a pass over the
> >>>> client_lru list and calls some function.  The next few patches will
> >>>> update other operations to use this function as well.  A value of 0
> >>>> still means "forget everything that is found".
> >>>>
> >>>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com> ---
> >>>> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++----- 1 file changed, 18
> >>>> insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
> >>>> 050a35e..07abca5 100644 --- a/fs/nfsd/nfs4state.c +++
> >>>> b/fs/nfsd/nfs4state.c @@ -4591,19 +4591,32 @@
> >>>> nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct
> >>>> nfsd_net *nn)
> >>>>  
> >>>>  #ifdef CONFIG_NFSD_FAULT_INJECTION
> >>>>  
> >>>> -void nfsd_forget_clients(u64 num) +u64 nfsd_forget_client(struct
> >>>> nfsd_net *nn, struct nfs4_client *clp, u64 max)
> >>>
> >>> It doesn't look like you really need nfsd_net?
> >>
> >> Not for this, but I find locks through the nn->ownerstr_hashtable
> >> since the nfs4_client doesn't have a "cl_locks" list similar to
> >> "cl_openowners" (unless there is something I'm missing...?).
> > 
> > That's because lockowners are all reachable from openowners (e.g. see
> > how it's done in
> > release_openowner->unhash_openowner->release_open_stateid->unhash_open_stateid->release_stateid_lockowners.
> > Yeah, it's a little tangled.).
> > 
> > Alternatively you can get to the network namespace from the client
> > (clp->net).
> 
> Both are good to know!  I'll look at getting lockowners from openowners first.

OK.  I've also updated my for-3.8 with more of Stanislav's
patches--doesn't look like there should be any interesting new
conflicts, but please check.

--b.
--
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
J. Bruce Fields Nov. 28, 2012, 4:57 p.m. UTC | #6
On Wed, Nov 28, 2012 at 11:56:42AM -0500, J. Bruce Fields wrote:
> On Wed, Nov 28, 2012 at 11:49:53AM -0500, Bryan Schumaker wrote:
> > On 11/28/2012 11:47 AM, J. Bruce Fields wrote:
> > > On Wed, Nov 28, 2012 at 11:34:40AM -0500, Bryan Schumaker wrote:
> > >> On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
> > >>> On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
> > >>>> From: Bryan Schumaker <bjschuma@netapp.com>
> > >>>>
> > >>>> I added in a generic for-each loop that takes a pass over the
> > >>>> client_lru list and calls some function.  The next few patches will
> > >>>> update other operations to use this function as well.  A value of 0
> > >>>> still means "forget everything that is found".
> > >>>>
> > >>>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com> ---
> > >>>> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++----- 1 file changed, 18
> > >>>> insertions(+), 5 deletions(-)
> > >>>>
> > >>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
> > >>>> 050a35e..07abca5 100644 --- a/fs/nfsd/nfs4state.c +++
> > >>>> b/fs/nfsd/nfs4state.c @@ -4591,19 +4591,32 @@
> > >>>> nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct
> > >>>> nfsd_net *nn)
> > >>>>  
> > >>>>  #ifdef CONFIG_NFSD_FAULT_INJECTION
> > >>>>  
> > >>>> -void nfsd_forget_clients(u64 num) +u64 nfsd_forget_client(struct
> > >>>> nfsd_net *nn, struct nfs4_client *clp, u64 max)
> > >>>
> > >>> It doesn't look like you really need nfsd_net?
> > >>
> > >> Not for this, but I find locks through the nn->ownerstr_hashtable
> > >> since the nfs4_client doesn't have a "cl_locks" list similar to
> > >> "cl_openowners" (unless there is something I'm missing...?).
> > > 
> > > That's because lockowners are all reachable from openowners (e.g. see
> > > how it's done in
> > > release_openowner->unhash_openowner->release_open_stateid->unhash_open_stateid->release_stateid_lockowners.
> > > Yeah, it's a little tangled.).
> > > 
> > > Alternatively you can get to the network namespace from the client
> > > (clp->net).
> > 
> > Both are good to know!  I'll look at getting lockowners from openowners first.
> 
> OK.  I've also updated my for-3.8 with more of Stanislav's
> patches--doesn't look like there should be any interesting new
> conflicts, but please check.

Oh, and maybe you can respin with that last bugfix folded in to the
right place, if that makes sense?

--b.
--
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
Bryan Schumaker Nov. 28, 2012, 4:57 p.m. UTC | #7
On 11/28/2012 11:56 AM, J. Bruce Fields wrote:
> On Wed, Nov 28, 2012 at 11:49:53AM -0500, Bryan Schumaker wrote:
>> On 11/28/2012 11:47 AM, J. Bruce Fields wrote:
>>> On Wed, Nov 28, 2012 at 11:34:40AM -0500, Bryan Schumaker wrote:
>>>> On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
>>>>> On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
>>>>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>>>>>
>>>>>> I added in a generic for-each loop that takes a pass over the
>>>>>> client_lru list and calls some function.  The next few patches will
>>>>>> update other operations to use this function as well.  A value of 0
>>>>>> still means "forget everything that is found".
>>>>>>
>>>>>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com> ---
>>>>>> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++----- 1 file changed, 18
>>>>>> insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
>>>>>> 050a35e..07abca5 100644 --- a/fs/nfsd/nfs4state.c +++
>>>>>> b/fs/nfsd/nfs4state.c @@ -4591,19 +4591,32 @@
>>>>>> nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct
>>>>>> nfsd_net *nn)
>>>>>>  
>>>>>>  #ifdef CONFIG_NFSD_FAULT_INJECTION
>>>>>>  
>>>>>> -void nfsd_forget_clients(u64 num) +u64 nfsd_forget_client(struct
>>>>>> nfsd_net *nn, struct nfs4_client *clp, u64 max)
>>>>>
>>>>> It doesn't look like you really need nfsd_net?
>>>>
>>>> Not for this, but I find locks through the nn->ownerstr_hashtable
>>>> since the nfs4_client doesn't have a "cl_locks" list similar to
>>>> "cl_openowners" (unless there is something I'm missing...?).
>>>
>>> That's because lockowners are all reachable from openowners (e.g. see
>>> how it's done in
>>> release_openowner->unhash_openowner->release_open_stateid->unhash_open_stateid->release_stateid_lockowners.
>>> Yeah, it's a little tangled.).
>>>
>>> Alternatively you can get to the network namespace from the client
>>> (clp->net).
>>
>> Both are good to know!  I'll look at getting lockowners from openowners first.
> 
> OK.  I've also updated my for-3.8 with more of Stanislav's
> patches--doesn't look like there should be any interesting new
> conflicts, but please check.

Will do.  I'll also work in Sasha Levin's patch from yesterday while I'm updating things.

- Bryan
> 
> --b.
> 

--
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
Bryan Schumaker Nov. 28, 2012, 8:53 p.m. UTC | #8
On 11/28/2012 11:57 AM, J. Bruce Fields wrote:
> On Wed, Nov 28, 2012 at 11:56:42AM -0500, J. Bruce Fields wrote:
>> On Wed, Nov 28, 2012 at 11:49:53AM -0500, Bryan Schumaker wrote:
>>> On 11/28/2012 11:47 AM, J. Bruce Fields wrote:
>>>> On Wed, Nov 28, 2012 at 11:34:40AM -0500, Bryan Schumaker wrote:
>>>>> On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
>>>>>> On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
>>>>>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>>>>>>
>>>>>>> I added in a generic for-each loop that takes a pass over the
>>>>>>> client_lru list and calls some function.  The next few patches will
>>>>>>> update other operations to use this function as well.  A value of 0
>>>>>>> still means "forget everything that is found".
>>>>>>>
>>>>>>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com> ---
>>>>>>> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++----- 1 file changed, 18
>>>>>>> insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
>>>>>>> 050a35e..07abca5 100644 --- a/fs/nfsd/nfs4state.c +++
>>>>>>> b/fs/nfsd/nfs4state.c @@ -4591,19 +4591,32 @@
>>>>>>> nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct
>>>>>>> nfsd_net *nn)
>>>>>>>  
>>>>>>>  #ifdef CONFIG_NFSD_FAULT_INJECTION
>>>>>>>  
>>>>>>> -void nfsd_forget_clients(u64 num) +u64 nfsd_forget_client(struct
>>>>>>> nfsd_net *nn, struct nfs4_client *clp, u64 max)
>>>>>>
>>>>>> It doesn't look like you really need nfsd_net?
>>>>>
>>>>> Not for this, but I find locks through the nn->ownerstr_hashtable
>>>>> since the nfs4_client doesn't have a "cl_locks" list similar to
>>>>> "cl_openowners" (unless there is something I'm missing...?).
>>>>
>>>> That's because lockowners are all reachable from openowners (e.g. see
>>>> how it's done in
>>>> release_openowner->unhash_openowner->release_open_stateid->unhash_open_stateid->release_stateid_lockowners.
>>>> Yeah, it's a little tangled.).
>>>>
>>>> Alternatively you can get to the network namespace from the client
>>>> (clp->net).
>>>
>>> Both are good to know!  I'll look at getting lockowners from openowners first.
>>
>> OK.  I've also updated my for-3.8 with more of Stanislav's
>> patches--doesn't look like there should be any interesting new
>> conflicts, but please check.

I've got something with multiple levels of list_for_each() loops that I'm attempting to unroll.  I just noticed that I've been releasing lock owners and open owners this whole time.  I wonder if it would be better to release lock and open stateids instead?

- Bryan 

> 
> Oh, and maybe you can respin with that last bugfix folded in to the
> right place, if that makes sense?
> 
> --b.
> 

--
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
J. Bruce Fields Nov. 30, 2012, 6:40 p.m. UTC | #9
On Wed, Nov 28, 2012 at 03:53:33PM -0500, Bryan Schumaker wrote:
> On 11/28/2012 11:57 AM, J. Bruce Fields wrote:
> > On Wed, Nov 28, 2012 at 11:56:42AM -0500, J. Bruce Fields wrote:
> >> On Wed, Nov 28, 2012 at 11:49:53AM -0500, Bryan Schumaker wrote:
> >>> On 11/28/2012 11:47 AM, J. Bruce Fields wrote:
> >>>> On Wed, Nov 28, 2012 at 11:34:40AM -0500, Bryan Schumaker wrote:
> >>>>> On 11/28/2012 11:29 AM, J. Bruce Fields wrote:
> >>>>>> On Tue, Nov 27, 2012 at 09:35:12AM -0500, bjschuma@netapp.com wrote:
> >>>>>>> From: Bryan Schumaker <bjschuma@netapp.com>
> >>>>>>>
> >>>>>>> I added in a generic for-each loop that takes a pass over the
> >>>>>>> client_lru list and calls some function.  The next few patches will
> >>>>>>> update other operations to use this function as well.  A value of 0
> >>>>>>> still means "forget everything that is found".
> >>>>>>>
> >>>>>>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com> ---
> >>>>>>> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++----- 1 file changed, 18
> >>>>>>> insertions(+), 5 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
> >>>>>>> 050a35e..07abca5 100644 --- a/fs/nfsd/nfs4state.c +++
> >>>>>>> b/fs/nfsd/nfs4state.c @@ -4591,19 +4591,32 @@
> >>>>>>> nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct
> >>>>>>> nfsd_net *nn)
> >>>>>>>  
> >>>>>>>  #ifdef CONFIG_NFSD_FAULT_INJECTION
> >>>>>>>  
> >>>>>>> -void nfsd_forget_clients(u64 num) +u64 nfsd_forget_client(struct
> >>>>>>> nfsd_net *nn, struct nfs4_client *clp, u64 max)
> >>>>>>
> >>>>>> It doesn't look like you really need nfsd_net?
> >>>>>
> >>>>> Not for this, but I find locks through the nn->ownerstr_hashtable
> >>>>> since the nfs4_client doesn't have a "cl_locks" list similar to
> >>>>> "cl_openowners" (unless there is something I'm missing...?).
> >>>>
> >>>> That's because lockowners are all reachable from openowners (e.g. see
> >>>> how it's done in
> >>>> release_openowner->unhash_openowner->release_open_stateid->unhash_open_stateid->release_stateid_lockowners.
> >>>> Yeah, it's a little tangled.).
> >>>>
> >>>> Alternatively you can get to the network namespace from the client
> >>>> (clp->net).
> >>>
> >>> Both are good to know!  I'll look at getting lockowners from openowners first.
> >>
> >> OK.  I've also updated my for-3.8 with more of Stanislav's
> >> patches--doesn't look like there should be any interesting new
> >> conflicts, but please check.
> 
> I've got something with multiple levels of list_for_each() loops that I'm attempting to unroll.  I just noticed that I've been releasing lock owners and open owners this whole time.  I wonder if it would be better to release lock and open stateids instead?

Could be, I don't know.  I think stateid's always have well-defined
lifetimes, don't they?  Whereas (at least in the 4.0 case?), it may be
up to the server to decide when exactly an open owner goes away.  If
that's right, counting stateid's may be more likely to give you
reproduceable test results.

--b.
--
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/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 050a35e..07abca5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4591,19 +4591,32 @@  nfs4_check_open_reclaim(clientid_t *clid, bool sessions, struct nfsd_net *nn)
 
 #ifdef CONFIG_NFSD_FAULT_INJECTION
 
-void nfsd_forget_clients(u64 num)
+u64 nfsd_forget_client(struct nfsd_net *nn, struct nfs4_client *clp, u64 max)
+{
+	nfsd4_client_record_remove(clp);
+	expire_client(clp);
+	return 1;
+}
+
+static u64 nfsd_for_n_state(u64 max, u64 (*func)(struct nfsd_net *, struct nfs4_client *, u64))
 {
 	struct nfs4_client *clp, *next;
-	int count = 0;
+	u64 count = 0;
 	struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id);
 
 	list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) {
-		expire_client(clp);
-		if (++count == num)
+		count += func(nn, clp, max - count);
+		if ((max != 0) && (count >= max))
 			break;
 	}
 
-	printk(KERN_INFO "NFSD: Forgot %d clients", count);
+	return count;
+}
+
+void nfsd_forget_clients(u64 num)
+{
+	u64 count = nfsd_for_n_state(num, nfsd_forget_client);
+	printk(KERN_INFO "NFSD: Forgot %llu clients", count);
 }
 
 static void release_lockowner_sop(struct nfs4_stateowner *sop)