* [PATCH] NFSd: fix locking in nfsd_forget_delegations() @ 2012-05-22 10:25 Stanislav Kinsbursky 2012-05-23 21:31 ` J. Bruce Fields 0 siblings, 1 reply; 5+ messages in thread From: Stanislav Kinsbursky @ 2012-05-22 10:25 UTC (permalink / raw) To: bfields; +Cc: linux-nfs, linux-kernel, devel This patch adds recall_lock hold to nfsd_forget_delegations() to protect nfsd_process_n_delegations() call. Also, looks like it would be better to collect delegations to some local on-stack list, and then unhash collected list. This split allows to simplify locking, because delegation traversing is protected by recall_lock, when delegation unhash is protected by client_mutex. Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com> --- fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++-------- 1 files changed, 24 insertions(+), 8 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 21266c7..f004e61 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -2597,7 +2597,7 @@ out: return ret; } -static void nfsd_break_one_deleg(struct nfs4_delegation *dp) +static void nfsd_break_one_deleg(struct nfs4_delegation *dp, void *data) { /* We're assuming the state code never drops its reference * without first removing the lease. Since we're in this lease @@ -2633,7 +2633,7 @@ static void nfsd_break_deleg_cb(struct file_lock *fl) spin_lock(&recall_lock); fp->fi_had_conflict = true; list_for_each_entry(dp, &fp->fi_delegations, dl_perfile) - nfsd_break_one_deleg(dp); + nfsd_break_one_deleg(dp, NULL); spin_unlock(&recall_lock); } @@ -4694,7 +4694,7 @@ void nfsd_forget_openowners(u64 num) printk(KERN_INFO "NFSD: Forgot %d open owners", count); } -int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *)) +int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *, void *), void *data) { int i, count = 0; struct nfs4_file *fp, *fnext; @@ -4703,7 +4703,7 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio for (i = 0; i < FILE_HASH_SIZE; i++) { list_for_each_entry_safe(fp, fnext, &file_hashtbl[i], fi_hash) { list_for_each_entry_safe(dp, dnext, &fp->fi_delegations, dl_perfile) { - deleg_func(dp); + deleg_func(dp, data); if (++count == num) return count; } @@ -4713,15 +4713,31 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio return count; } +/* Called under the recall_lock spinlock. */ +static void +collect_delegation(struct nfs4_delegation *dp, void *data) +{ + struct list_head *list = data; + + list_move(&dp->dl_perfile, list); +} + void nfsd_forget_delegations(u64 num) { unsigned int count; + struct nfs4_delegation *dp, *dnext; + LIST_HEAD(unhash_list); - nfs4_lock_state(); - count = nfsd_process_n_delegations(num, unhash_delegation); - nfs4_unlock_state(); + spin_lock(&recall_lock); + count = nfsd_process_n_delegations(num, collect_delegation, &unhash_list); + spin_unlock(&recall_lock); printk(KERN_INFO "NFSD: Forgot %d delegations", count); + + nfs4_lock_state(); + list_for_each_entry_safe(dp, dnext, &unhash_list, dl_perfile) + unhash_delegation(dp); + nfs4_unlock_state(); } void nfsd_recall_delegations(u64 num) @@ -4730,7 +4746,7 @@ void nfsd_recall_delegations(u64 num) nfs4_lock_state(); spin_lock(&recall_lock); - count = nfsd_process_n_delegations(num, nfsd_break_one_deleg); + count = nfsd_process_n_delegations(num, nfsd_break_one_deleg, NULL); spin_unlock(&recall_lock); nfs4_unlock_state(); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] NFSd: fix locking in nfsd_forget_delegations() 2012-05-22 10:25 [PATCH] NFSd: fix locking in nfsd_forget_delegations() Stanislav Kinsbursky @ 2012-05-23 21:31 ` J. Bruce Fields 2012-05-24 4:41 ` Stanislav Kinsbursky 0 siblings, 1 reply; 5+ messages in thread From: J. Bruce Fields @ 2012-05-23 21:31 UTC (permalink / raw) To: Stanislav Kinsbursky; +Cc: linux-nfs, linux-kernel, devel On Tue, May 22, 2012 at 02:25:14PM +0400, Stanislav Kinsbursky wrote: > This patch adds recall_lock hold to nfsd_forget_delegations() to protect > nfsd_process_n_delegations() call. > Also, looks like it would be better to collect delegations to some local > on-stack list, and then unhash collected list. This split allows to simplify > locking, because delegation traversing is protected by recall_lock, when > delegation unhash is protected by client_mutex. All this indirection is getting a little much. How about replacing nfsd_process_n_delegations by something that always does the list-move?: void nfsd_forget_delegations(u64 num) { unsigned int count; list_head victims; nfs4_lock_state(); count = nfsd_get_n_delegations(num, &victims); list_for_each_entry_safe(..., &victims, ...) unhash_delegation(); unlock_state(); } ditto for recall_delegations, and take the recall_lock inside nfsd_get_n_delegations? Or something like that. --b. > > Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com> > --- > fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++-------- > 1 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 21266c7..f004e61 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -2597,7 +2597,7 @@ out: > return ret; > } > > -static void nfsd_break_one_deleg(struct nfs4_delegation *dp) > +static void nfsd_break_one_deleg(struct nfs4_delegation *dp, void *data) > { > /* We're assuming the state code never drops its reference > * without first removing the lease. Since we're in this lease > @@ -2633,7 +2633,7 @@ static void nfsd_break_deleg_cb(struct file_lock *fl) > spin_lock(&recall_lock); > fp->fi_had_conflict = true; > list_for_each_entry(dp, &fp->fi_delegations, dl_perfile) > - nfsd_break_one_deleg(dp); > + nfsd_break_one_deleg(dp, NULL); > spin_unlock(&recall_lock); > } > > @@ -4694,7 +4694,7 @@ void nfsd_forget_openowners(u64 num) > printk(KERN_INFO "NFSD: Forgot %d open owners", count); > } > > -int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *)) > +int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *, void *), void *data) > { > int i, count = 0; > struct nfs4_file *fp, *fnext; > @@ -4703,7 +4703,7 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio > for (i = 0; i < FILE_HASH_SIZE; i++) { > list_for_each_entry_safe(fp, fnext, &file_hashtbl[i], fi_hash) { > list_for_each_entry_safe(dp, dnext, &fp->fi_delegations, dl_perfile) { > - deleg_func(dp); > + deleg_func(dp, data); > if (++count == num) > return count; > } > @@ -4713,15 +4713,31 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio > return count; > } > > +/* Called under the recall_lock spinlock. */ > +static void > +collect_delegation(struct nfs4_delegation *dp, void *data) > +{ > + struct list_head *list = data; > + > + list_move(&dp->dl_perfile, list); > +} > + > void nfsd_forget_delegations(u64 num) > { > unsigned int count; > + struct nfs4_delegation *dp, *dnext; > + LIST_HEAD(unhash_list); > > - nfs4_lock_state(); > - count = nfsd_process_n_delegations(num, unhash_delegation); > - nfs4_unlock_state(); > + spin_lock(&recall_lock); > + count = nfsd_process_n_delegations(num, collect_delegation, &unhash_list); > + spin_unlock(&recall_lock); > > printk(KERN_INFO "NFSD: Forgot %d delegations", count); > + > + nfs4_lock_state(); > + list_for_each_entry_safe(dp, dnext, &unhash_list, dl_perfile) > + unhash_delegation(dp); > + nfs4_unlock_state(); > } > > void nfsd_recall_delegations(u64 num) > @@ -4730,7 +4746,7 @@ void nfsd_recall_delegations(u64 num) > > nfs4_lock_state(); > spin_lock(&recall_lock); > - count = nfsd_process_n_delegations(num, nfsd_break_one_deleg); > + count = nfsd_process_n_delegations(num, nfsd_break_one_deleg, NULL); > spin_unlock(&recall_lock); > nfs4_unlock_state(); > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] NFSd: fix locking in nfsd_forget_delegations() 2012-05-23 21:31 ` J. Bruce Fields @ 2012-05-24 4:41 ` Stanislav Kinsbursky 2012-05-24 10:56 ` J. Bruce Fields 0 siblings, 1 reply; 5+ messages in thread From: Stanislav Kinsbursky @ 2012-05-24 4:41 UTC (permalink / raw) To: J. Bruce Fields Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org 24.05.2012 01:31, J. Bruce Fields написал: > On Tue, May 22, 2012 at 02:25:14PM +0400, Stanislav Kinsbursky wrote: >> This patch adds recall_lock hold to nfsd_forget_delegations() to protect >> nfsd_process_n_delegations() call. >> Also, looks like it would be better to collect delegations to some local >> on-stack list, and then unhash collected list. This split allows to simplify >> locking, because delegation traversing is protected by recall_lock, when >> delegation unhash is protected by client_mutex. > All this indirection is getting a little much. > > How about replacing nfsd_process_n_delegations by something that always > does the list-move?: Is it correct? List move is suitable for unhash delegations since we anyway remove delegation from fi_delegations list. But seems we don't do this for delegations recall... > void nfsd_forget_delegations(u64 num) > { > unsigned int count; > list_head victims; > > nfs4_lock_state(); > count = nfsd_get_n_delegations(num,&victims); > list_for_each_entry_safe(...,&victims, ...) > unhash_delegation(); > unlock_state(); > } > > ditto for recall_delegations, and take the recall_lock inside > nfsd_get_n_delegations? > > Or something like that. > > --b. > >> Signed-off-by: Stanislav Kinsbursky<skinsbursky@parallels.com> >> --- >> fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++-------- >> 1 files changed, 24 insertions(+), 8 deletions(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index 21266c7..f004e61 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -2597,7 +2597,7 @@ out: >> return ret; >> } >> >> -static void nfsd_break_one_deleg(struct nfs4_delegation *dp) >> +static void nfsd_break_one_deleg(struct nfs4_delegation *dp, void *data) >> { >> /* We're assuming the state code never drops its reference >> * without first removing the lease. Since we're in this lease >> @@ -2633,7 +2633,7 @@ static void nfsd_break_deleg_cb(struct file_lock *fl) >> spin_lock(&recall_lock); >> fp->fi_had_conflict = true; >> list_for_each_entry(dp,&fp->fi_delegations, dl_perfile) >> - nfsd_break_one_deleg(dp); >> + nfsd_break_one_deleg(dp, NULL); >> spin_unlock(&recall_lock); >> } >> >> @@ -4694,7 +4694,7 @@ void nfsd_forget_openowners(u64 num) >> printk(KERN_INFO "NFSD: Forgot %d open owners", count); >> } >> >> -int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *)) >> +int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *, void *), void *data) >> { >> int i, count = 0; >> struct nfs4_file *fp, *fnext; >> @@ -4703,7 +4703,7 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio >> for (i = 0; i< FILE_HASH_SIZE; i++) { >> list_for_each_entry_safe(fp, fnext,&file_hashtbl[i], fi_hash) { >> list_for_each_entry_safe(dp, dnext,&fp->fi_delegations, dl_perfile) { >> - deleg_func(dp); >> + deleg_func(dp, data); >> if (++count == num) >> return count; >> } >> @@ -4713,15 +4713,31 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio >> return count; >> } >> >> +/* Called under the recall_lock spinlock. */ >> +static void >> +collect_delegation(struct nfs4_delegation *dp, void *data) >> +{ >> + struct list_head *list = data; >> + >> + list_move(&dp->dl_perfile, list); >> +} >> + >> void nfsd_forget_delegations(u64 num) >> { >> unsigned int count; >> + struct nfs4_delegation *dp, *dnext; >> + LIST_HEAD(unhash_list); >> >> - nfs4_lock_state(); >> - count = nfsd_process_n_delegations(num, unhash_delegation); >> - nfs4_unlock_state(); >> + spin_lock(&recall_lock); >> + count = nfsd_process_n_delegations(num, collect_delegation,&unhash_list); >> + spin_unlock(&recall_lock); >> >> printk(KERN_INFO "NFSD: Forgot %d delegations", count); >> + >> + nfs4_lock_state(); >> + list_for_each_entry_safe(dp, dnext,&unhash_list, dl_perfile) >> + unhash_delegation(dp); >> + nfs4_unlock_state(); >> } >> >> void nfsd_recall_delegations(u64 num) >> @@ -4730,7 +4746,7 @@ void nfsd_recall_delegations(u64 num) >> >> nfs4_lock_state(); >> spin_lock(&recall_lock); >> - count = nfsd_process_n_delegations(num, nfsd_break_one_deleg); >> + count = nfsd_process_n_delegations(num, nfsd_break_one_deleg, NULL); >> spin_unlock(&recall_lock); >> nfs4_unlock_state(); >> >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] NFSd: fix locking in nfsd_forget_delegations() 2012-05-24 4:41 ` Stanislav Kinsbursky @ 2012-05-24 10:56 ` J. Bruce Fields 2012-05-24 11:09 ` Stanislav Kinsbursky 0 siblings, 1 reply; 5+ messages in thread From: J. Bruce Fields @ 2012-05-24 10:56 UTC (permalink / raw) To: Stanislav Kinsbursky Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org On Thu, May 24, 2012 at 08:41:35AM +0400, Stanislav Kinsbursky wrote: > 24.05.2012 01:31, J. Bruce Fields написал: > >On Tue, May 22, 2012 at 02:25:14PM +0400, Stanislav Kinsbursky wrote: > >>This patch adds recall_lock hold to nfsd_forget_delegations() to protect > >>nfsd_process_n_delegations() call. > >>Also, looks like it would be better to collect delegations to some local > >>on-stack list, and then unhash collected list. This split allows to simplify > >>locking, because delegation traversing is protected by recall_lock, when > >>delegation unhash is protected by client_mutex. > >All this indirection is getting a little much. > > > >How about replacing nfsd_process_n_delegations by something that always > >does the list-move?: > > Is it correct? > List move is suitable for unhash delegations since we anyway remove > delegation from fi_delegations list. > But seems we don't do this for delegations recall... Oh, blah, you're right of course. Still, this seems a little tangled, and I'd prefer not to have to add the useless extra parameter to break_one_deleg(). --b. > > > >void nfsd_forget_delegations(u64 num) > >{ > > unsigned int count; > > list_head victims; > > > > nfs4_lock_state(); > > count = nfsd_get_n_delegations(num,&victims); > > list_for_each_entry_safe(...,&victims, ...) > > unhash_delegation(); > > unlock_state(); > >} > > > >ditto for recall_delegations, and take the recall_lock inside > >nfsd_get_n_delegations? > > > >Or something like that. > > > >--b. > > > >>Signed-off-by: Stanislav Kinsbursky<skinsbursky@parallels.com> > >>--- > >> fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++-------- > >> 1 files changed, 24 insertions(+), 8 deletions(-) > >> > >>diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > >>index 21266c7..f004e61 100644 > >>--- a/fs/nfsd/nfs4state.c > >>+++ b/fs/nfsd/nfs4state.c > >>@@ -2597,7 +2597,7 @@ out: > >> return ret; > >> } > >> > >>-static void nfsd_break_one_deleg(struct nfs4_delegation *dp) > >>+static void nfsd_break_one_deleg(struct nfs4_delegation *dp, void *data) > >> { > >> /* We're assuming the state code never drops its reference > >> * without first removing the lease. Since we're in this lease > >>@@ -2633,7 +2633,7 @@ static void nfsd_break_deleg_cb(struct file_lock *fl) > >> spin_lock(&recall_lock); > >> fp->fi_had_conflict = true; > >> list_for_each_entry(dp,&fp->fi_delegations, dl_perfile) > >>- nfsd_break_one_deleg(dp); > >>+ nfsd_break_one_deleg(dp, NULL); > >> spin_unlock(&recall_lock); > >> } > >> > >>@@ -4694,7 +4694,7 @@ void nfsd_forget_openowners(u64 num) > >> printk(KERN_INFO "NFSD: Forgot %d open owners", count); > >> } > >> > >>-int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *)) > >>+int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *, void *), void *data) > >> { > >> int i, count = 0; > >> struct nfs4_file *fp, *fnext; > >>@@ -4703,7 +4703,7 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio > >> for (i = 0; i< FILE_HASH_SIZE; i++) { > >> list_for_each_entry_safe(fp, fnext,&file_hashtbl[i], fi_hash) { > >> list_for_each_entry_safe(dp, dnext,&fp->fi_delegations, dl_perfile) { > >>- deleg_func(dp); > >>+ deleg_func(dp, data); > >> if (++count == num) > >> return count; > >> } > >>@@ -4713,15 +4713,31 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio > >> return count; > >> } > >> > >>+/* Called under the recall_lock spinlock. */ > >>+static void > >>+collect_delegation(struct nfs4_delegation *dp, void *data) > >>+{ > >>+ struct list_head *list = data; > >>+ > >>+ list_move(&dp->dl_perfile, list); > >>+} > >>+ > >> void nfsd_forget_delegations(u64 num) > >> { > >> unsigned int count; > >>+ struct nfs4_delegation *dp, *dnext; > >>+ LIST_HEAD(unhash_list); > >> > >>- nfs4_lock_state(); > >>- count = nfsd_process_n_delegations(num, unhash_delegation); > >>- nfs4_unlock_state(); > >>+ spin_lock(&recall_lock); > >>+ count = nfsd_process_n_delegations(num, collect_delegation,&unhash_list); > >>+ spin_unlock(&recall_lock); > >> > >> printk(KERN_INFO "NFSD: Forgot %d delegations", count); > >>+ > >>+ nfs4_lock_state(); > >>+ list_for_each_entry_safe(dp, dnext,&unhash_list, dl_perfile) > >>+ unhash_delegation(dp); > >>+ nfs4_unlock_state(); > >> } > >> > >> void nfsd_recall_delegations(u64 num) > >>@@ -4730,7 +4746,7 @@ void nfsd_recall_delegations(u64 num) > >> > >> nfs4_lock_state(); > >> spin_lock(&recall_lock); > >>- count = nfsd_process_n_delegations(num, nfsd_break_one_deleg); > >>+ count = nfsd_process_n_delegations(num, nfsd_break_one_deleg, NULL); > >> spin_unlock(&recall_lock); > >> nfs4_unlock_state(); > >> > >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] NFSd: fix locking in nfsd_forget_delegations() 2012-05-24 10:56 ` J. Bruce Fields @ 2012-05-24 11:09 ` Stanislav Kinsbursky 0 siblings, 0 replies; 5+ messages in thread From: Stanislav Kinsbursky @ 2012-05-24 11:09 UTC (permalink / raw) To: J. Bruce Fields Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, devel@openvz.org On 24.05.2012 14:56, J. Bruce Fields wrote: > On Thu, May 24, 2012 at 08:41:35AM +0400, Stanislav Kinsbursky wrote: >> 24.05.2012 01:31, J. Bruce Fields написал: >>> On Tue, May 22, 2012 at 02:25:14PM +0400, Stanislav Kinsbursky wrote: >>>> This patch adds recall_lock hold to nfsd_forget_delegations() to protect >>>> nfsd_process_n_delegations() call. >>>> Also, looks like it would be better to collect delegations to some local >>>> on-stack list, and then unhash collected list. This split allows to simplify >>>> locking, because delegation traversing is protected by recall_lock, when >>>> delegation unhash is protected by client_mutex. >>> All this indirection is getting a little much. >>> >>> How about replacing nfsd_process_n_delegations by something that always >>> does the list-move?: >> >> Is it correct? >> List move is suitable for unhash delegations since we anyway remove >> delegation from fi_delegations list. >> But seems we don't do this for delegations recall... > > Oh, blah, you're right of course. > > Still, this seems a little tangled, and I'd prefer not to have to add > the useless extra parameter to break_one_deleg(). > Ok, I'll try to handle it somehow... > --b. > >> >> >>> void nfsd_forget_delegations(u64 num) >>> { >>> unsigned int count; >>> list_head victims; >>> >>> nfs4_lock_state(); >>> count = nfsd_get_n_delegations(num,&victims); >>> list_for_each_entry_safe(...,&victims, ...) >>> unhash_delegation(); >>> unlock_state(); >>> } >>> >>> ditto for recall_delegations, and take the recall_lock inside >>> nfsd_get_n_delegations? >>> >>> Or something like that. >>> >>> --b. >>> >>>> Signed-off-by: Stanislav Kinsbursky<skinsbursky@parallels.com> >>>> --- >>>> fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++-------- >>>> 1 files changed, 24 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>>> index 21266c7..f004e61 100644 >>>> --- a/fs/nfsd/nfs4state.c >>>> +++ b/fs/nfsd/nfs4state.c >>>> @@ -2597,7 +2597,7 @@ out: >>>> return ret; >>>> } >>>> >>>> -static void nfsd_break_one_deleg(struct nfs4_delegation *dp) >>>> +static void nfsd_break_one_deleg(struct nfs4_delegation *dp, void *data) >>>> { >>>> /* We're assuming the state code never drops its reference >>>> * without first removing the lease. Since we're in this lease >>>> @@ -2633,7 +2633,7 @@ static void nfsd_break_deleg_cb(struct file_lock *fl) >>>> spin_lock(&recall_lock); >>>> fp->fi_had_conflict = true; >>>> list_for_each_entry(dp,&fp->fi_delegations, dl_perfile) >>>> - nfsd_break_one_deleg(dp); >>>> + nfsd_break_one_deleg(dp, NULL); >>>> spin_unlock(&recall_lock); >>>> } >>>> >>>> @@ -4694,7 +4694,7 @@ void nfsd_forget_openowners(u64 num) >>>> printk(KERN_INFO "NFSD: Forgot %d open owners", count); >>>> } >>>> >>>> -int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *)) >>>> +int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *, void *), void *data) >>>> { >>>> int i, count = 0; >>>> struct nfs4_file *fp, *fnext; >>>> @@ -4703,7 +4703,7 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio >>>> for (i = 0; i< FILE_HASH_SIZE; i++) { >>>> list_for_each_entry_safe(fp, fnext,&file_hashtbl[i], fi_hash) { >>>> list_for_each_entry_safe(dp, dnext,&fp->fi_delegations, dl_perfile) { >>>> - deleg_func(dp); >>>> + deleg_func(dp, data); >>>> if (++count == num) >>>> return count; >>>> } >>>> @@ -4713,15 +4713,31 @@ int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegatio >>>> return count; >>>> } >>>> >>>> +/* Called under the recall_lock spinlock. */ >>>> +static void >>>> +collect_delegation(struct nfs4_delegation *dp, void *data) >>>> +{ >>>> + struct list_head *list = data; >>>> + >>>> + list_move(&dp->dl_perfile, list); >>>> +} >>>> + >>>> void nfsd_forget_delegations(u64 num) >>>> { >>>> unsigned int count; >>>> + struct nfs4_delegation *dp, *dnext; >>>> + LIST_HEAD(unhash_list); >>>> >>>> - nfs4_lock_state(); >>>> - count = nfsd_process_n_delegations(num, unhash_delegation); >>>> - nfs4_unlock_state(); >>>> + spin_lock(&recall_lock); >>>> + count = nfsd_process_n_delegations(num, collect_delegation,&unhash_list); >>>> + spin_unlock(&recall_lock); >>>> >>>> printk(KERN_INFO "NFSD: Forgot %d delegations", count); >>>> + >>>> + nfs4_lock_state(); >>>> + list_for_each_entry_safe(dp, dnext,&unhash_list, dl_perfile) >>>> + unhash_delegation(dp); >>>> + nfs4_unlock_state(); >>>> } >>>> >>>> void nfsd_recall_delegations(u64 num) >>>> @@ -4730,7 +4746,7 @@ void nfsd_recall_delegations(u64 num) >>>> >>>> nfs4_lock_state(); >>>> spin_lock(&recall_lock); >>>> - count = nfsd_process_n_delegations(num, nfsd_break_one_deleg); >>>> + count = nfsd_process_n_delegations(num, nfsd_break_one_deleg, NULL); >>>> spin_unlock(&recall_lock); >>>> nfs4_unlock_state(); >>>> >>>> >> -- Best regards, Stanislav Kinsbursky ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-05-24 11:09 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-22 10:25 [PATCH] NFSd: fix locking in nfsd_forget_delegations() Stanislav Kinsbursky 2012-05-23 21:31 ` J. Bruce Fields 2012-05-24 4:41 ` Stanislav Kinsbursky 2012-05-24 10:56 ` J. Bruce Fields 2012-05-24 11:09 ` Stanislav Kinsbursky
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).