2011-10-25 12:19:25

by J. Bruce Fields

[permalink] [raw]
Subject: nfsd (and lock) changes for 3.2

Changes for nfsd (and also a few file-locking changes) are available in
the git repository at:

git://linux-nfs.org/~bfields/linux.git for-3.2

This is the same tree I've always used--I assume you believe it's mine
without a need for signing anything. Feel free to tell me otherwise.

A voice in my head says "wait wait X Y and Z are ready Any Day Now!",
and I'm silencing that voice by pretending I may send another (much
smaller) pull request in a week. Don't count on it. Anyway:

In this request, among other work:

- A rewrite of large parts of the NFSv4 state management to fix
bugs and make the code more mantainable. One of the goals was
to break up the big state lock--unfortunately I didn't get
there, but it looks close.

- Several 4.1 fixes from Benny Halevy--thanks to Benny for
testing the server at the fall bakeathon.

- Progress on the basic 4.1 todo's. Thanks in particular to Mi
Jinlong for (among other things) implementing the somewhat
tricky DRC limit checking.

The 4.1 code is getting closer--it *might* be possible to
finish basic 4.1 early as 3.3, at which point we could start
on optional features (like pNFS). But that will depend on
people sending patches (and pynfs tests) for the remaining 4.1
todo's.

- Fix for longstanding problems with duplicate directory cookies
from ext3/4, from Bernd Schubert. (One nfsv4-specific patch
only in this pull, I believe the rest is going through the
ext4 tree.)

- From Trond (and Chuck), kernel support for federated NFS.

- Fixes and some cleanup for vfs lease code. I've deferred a
larger change to fix lease semantics for NFSv4 after
discovering those "fixes" aren't actually correct for Samba
(the only other user I know of), and instead I intend to
introduce a new lock type for NFSv4's use.

Plus some other miscellaneous bug fixes--thanks to everyone who reported
a bug or contributed a patch!

--b.

Benny Halevy (4):
nfsd41: use SEQ4_STATUS_BACKCHANNEL_FAULT when cb_sequence is invalid
nfsd4: seq->status_flags may be used unitialized
nfsd4: allow NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL | NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED
nfsd4: typo logical vs bitwise negate for want_mask

Bernd Schubert (1):
nfsd4: Remove check for a 32-bit cookie in nfsd4_readdir()

Boaz Harrosh (1):
nfsd4: fix failure to end nfsd4 grace period

Dan Carpenter (1):
nfsd4: typo logical vs bitwise negate

Eric Dumazet (1):
sunrpc: use better NUMA affinities

J. Bruce Fields (86):
nfsd4: fix seqid_mutating_error
nfsd4: return nfserr_symlink on v4 OPEN of non-regular file
locks: minor lease cleanup
locks: move F_INPROGRESS from fl_type to fl_flags field
locks: fix tracking of inprogress lease breaks
locks: setlease cleanup
nfsd4: clean up S_IS -> NF4 file type mapping
nfsd: open-code special directory-hardlink check
nfsd: clean up nfsd_mode_check()
nfsd4: fix incorrect comment in nfsd4_set_nfs4_acl
nfsd4: it's OK to return nfserr_symlink
nfsd: remove unused defines
Remove include/linux/nfsd/const.h
nfsd4: permit read opens of executable-only files
nfsd: prettify NFSD_MAY_* flag definitions
nfsd4: simplify recovery dir setting
nfsd4: stop using nfserr_resource for transitory errors
nfsd4: replace some macros by functions
nfsd4: name openowner data structures more clearly
nfsd4: cleanup lock/stateowner initialization
nfsd4: remove HAS_SESSION
nfsd4: cleanup and consolidate seqid_mutating_err
nfsd4: simplify lock openmode check
nfsd4: get lock checks out of preprocess_seqid_op
nfsd4: remove redundant is_open_owner check
nfsd: remove include/linux/nfsd/syscall.h
nfsd4: fix off-by-one-error in SEQUENCE reply
nfsd4: remove typoed replay field
nfsd4: simplify distinguishing lock & open stateid's
nfsd4: consolidate lock & open stateid tables
nfsd4: simplify stateid generation code, fix wraparound
nfsd4: make delegation stateid's seqid start at 1
nfsd4: centralize handling of replay owners
nfsd4: cleanup seqid op stateowner usage
nfsd4: extend state lock over seqid replay logic
nfsd4: eliminate impossible open replay case
nfsd4: drop most stateowner refcounting
nfsd4: eliminate unused lt_stateowner
nfsd4: share common seqid checks
nfsd4: simplify check_open logic
nfsd4: move double-confirm test to open_confirm
nfsd4: move CLOSE_STATE special case to caller
nfsd4: split stateowners into open and lockowners
nfsd4: split out some free_generic_stateid code
nfsd4: rearrange to avoid a forward reference
nfsd4: split up find_stateid
nfsd4: split preprocess_seqid, cleanup
nfsd4: pass around typemask instead of flags
nfsd4: rename init_stateid
nfsd4: remove redundant stateid initialization
nfsd4: move some of nfs4_stateid into a separate structure
nfsd4: add common dl_stid field to delegation
nfsd4: share common stid-hashing helper function
nfsd4: hash deleg stateid's like any other
nfsd4: fix test_stateid for delegation stateid's
nfsd4: use deleg changes to cleanup preprocess_stateid_op
nfsd4: better stateid hashing
nfsd4: replace oo_confirmed by flag bit
nfsd4: match close replays on stateid, not open owner id
nfsd4: simplify free_stateid
nfsd4: construct stateid from clientid and counter
nfsd4: hash closed stateid's like any other
nfsd4: fix open downgrade, again
nfsd4: make op_cacheresult another flag
leases: split up generic_setlease into lock/unlock cases
nfsd4: move client * to nfs4_stateid, add init_stid helper
nfsd4: use idr for stateid's
nfsd4: assume test_stateid always has session
nfsd4: look up stateid's per clientid
nfsd4: fix state lock usage in LOCKU
nfsd4: clean up downgrading code
nfsd4: cleanup state.h comments
nfsd4: ignore WANT bits in open downgrade
nfsd4: move access/deny validity checks to xdr code
nfsd4: move name-length checks to xdr
nfsd4: more robust ignoring of WANT bits in OPEN
nfsd4: centralize renew_client() calls
nfsd4: make is_open_owner boolean
nfsd4: simplify process_open1 logic
nfsd4: clean up open owners on OPEN failure
nfsd4: preallocate nfs4_file in process_open1()
nfsd4: do idr preallocation with stateid allocation
nfsd4: preallocate open stateid in process_open1()
nfsd4: warn on open failure after create
nfsd4: remove unneeded CLAIM_DELEGATE_CUR workaround
nfsd4: implement new 4.1 open reclaim types

Mi Jinlong (5):
SUNRPC: Replace svc_addr_u by sockaddr_storage
SUNRPC: compare scopeid for link-local addresses
nfsd41: try to check reply size before operation
nfs: fix bug about IPv6 address scope checking
nfs41: implement DESTROY_CLIENTID operation

Michal Schmidt (1):
sunrpc: add MODULE_ALIAS to match the filesystem name

Trond Myklebust (3):
NFSD: Cleanup for nfsd4_path()
NFSD: Remove the ex_pathname field from struct svc_export
NFSD: Add a cache for fs_locations information

arch/alpha/include/asm/fcntl.h | 2 -
fs/compat.c | 1 -
fs/lockd/host.c | 25 +-
fs/lockd/svc.c | 2 +-
fs/locks.c | 223 ++++--
fs/nfs/callback.c | 4 +-
fs/nfs/client.c | 7 +-
fs/nfs/nfs4_fs.h | 24 -
fs/nfsd/export.c | 16 +-
fs/nfsd/nfs4callback.c | 20 +-
fs/nfsd/nfs4proc.c | 374 +++++++--
fs/nfsd/nfs4recover.c | 53 +-
fs/nfsd/nfs4state.c | 1794 +++++++++++++++++++---------------------
fs/nfsd/nfs4xdr.c | 380 ++++++---
fs/nfsd/nfsctl.c | 1 -
fs/nfsd/nfsd.h | 33 +
fs/nfsd/nfsfh.c | 39 +-
fs/nfsd/state.h | 174 ++--
fs/nfsd/vfs.c | 31 +-
fs/nfsd/vfs.h | 29 +-
fs/nfsd/xdr4.h | 28 +-
include/asm-generic/fcntl.h | 5 -
include/linux/fs.h | 8 +-
include/linux/nfs4.h | 21 +-
include/linux/nfsd/Kbuild | 2 -
include/linux/nfsd/const.h | 55 --
include/linux/nfsd/export.h | 2 +-
include/linux/nfsd/nfsfh.h | 7 +-
include/linux/nfsd/syscall.h | 116 ---
include/linux/sunrpc/clnt.h | 8 +-
include/linux/sunrpc/svc.h | 32 +-
net/sunrpc/rpc_pipe.c | 3 +
net/sunrpc/svc.c | 33 +-
net/sunrpc/svc_xprt.c | 13 +-
net/sunrpc/svcsock.c | 23 +-
35 files changed, 1901 insertions(+), 1687 deletions(-)
delete mode 100644 include/linux/nfsd/const.h
delete mode 100644 include/linux/nfsd/syscall.h


2011-10-26 08:59:41

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On Tue, Oct 25, 2011 at 11:41:43PM -0500, Tom Tucker wrote:
> Hi Bruce,
>
> What does RDMA Non Support mean exactly? Maybe I could help if it's
> a resource issue?

My notes are here:

http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#Clarify_RDMA_non-support

Feel free to edit that. (That also includes a pointer to some notes you
made on 4.1 and RDMA, though mainly focusing on the client side.)

Initially my minimal goal is just to make sure that the NFSv4.1 server
errors out cleanly at the start when a client attempts to use RDMA.

--b.

>
> Thanks,
> Tom
>
> On 10/25/11 7:34 AM, J. Bruce Fields wrote:
> >On Tue, Oct 25, 2011 at 08:19:24AM -0400, bfields wrote:
> >> - Progress on the basic 4.1 todo's. Thanks in particular to Mi
> >> Jinlong for (among other things) implementing the somewhat
> >> tricky DRC limit checking.
> >>
> >> The 4.1 code is getting closer--it *might* be possible to
> >> finish basic 4.1 early as 3.3, at which point we could start
> >> on optional features (like pNFS). But that will depend on
> >> people sending patches (and pynfs tests) for the remaining 4.1
> >> todo's.
> >I've been keeping the todo list up to date here:
> >
> > http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues
> >
> >Some of them I think are relatively straightforward--probably anyone
> >with a little time could dive into them:
> >
> > - SEQ4_STATUS_RECALLABLE_STATE_REVOKED
> >
> > - backchannel attribute negotiation
> >
> > - ACL retention bits
> >
> > - Clarify RDMA non-support
> >
> >Slightly trickier or open-ended; we may need to talk over design first
> >before diving in:
> >
> > - Make DESTROY_SESSION wait on in-progress requests
> >
> > - current stateid
> >
> > - Check 4.0/4.1 interactions
> >
> > - Callback failure handling
> >
> > - GSS
> >
> >The GSS piece is probably the one I'm most worried about. What we have
> >seems sufficient for current clients, but I know it's incomplete, and
> >I'm not entirely clear how much work it is to implement the minimum
> >required to ensure that future kerberos-using clients won't break
> >unexpectedly on upgrade to 4.1.
> >
> >--b.
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >the body of a message to [email protected]
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2011-10-26 03:16:24

by Boaz Harrosh

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On 10/25/2011 05:19 AM, J. Bruce Fields wrote:
> Changes for nfsd (and also a few file-locking changes) are available in
> the git repository at:
>
> git://linux-nfs.org/~bfields/linux.git for-3.2
>
> This is the same tree I've always used--I assume you believe it's mine
> without a need for signing anything. Feel free to tell me otherwise.
>

<Snip>

>
> Boaz Harrosh (1):
> nfsd4: fix failure to end nfsd4 grace period
>

Bruce hi.

As I recall this failure I have experienced in early stages of the
3.1-rcX series. I admit that I always run with Benny's pnfs tree,
but I do not think it is patches that Benny added from the 3.2 set
I think it is breakage introduced by the 3.1 merge window.

Therefor I think that this patch must be sent to Stable@ otherwise
surly 3.1 Kernel users are to hit it. (You can escapee it)

Though it has been pulled to Linus tree, without the CC: stable@kernel
I think you can just send the patch to <[email protected]> and it
will be included.

(What other 3.1 bug fixes are missing)

Thanks
Boaz

2011-10-25 12:34:50

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On Tue, Oct 25, 2011 at 08:19:24AM -0400, bfields wrote:
> - Progress on the basic 4.1 todo's. Thanks in particular to Mi
> Jinlong for (among other things) implementing the somewhat
> tricky DRC limit checking.
>
> The 4.1 code is getting closer--it *might* be possible to
> finish basic 4.1 early as 3.3, at which point we could start
> on optional features (like pNFS). But that will depend on
> people sending patches (and pynfs tests) for the remaining 4.1
> todo's.

I've been keeping the todo list up to date here:

http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues

Some of them I think are relatively straightforward--probably anyone
with a little time could dive into them:

- SEQ4_STATUS_RECALLABLE_STATE_REVOKED

- backchannel attribute negotiation

- ACL retention bits

- Clarify RDMA non-support

Slightly trickier or open-ended; we may need to talk over design first
before diving in:

- Make DESTROY_SESSION wait on in-progress requests

- current stateid

- Check 4.0/4.1 interactions

- Callback failure handling

- GSS

The GSS piece is probably the one I'm most worried about. What we have
seems sufficient for current clients, but I know it's incomplete, and
I'm not entirely clear how much work it is to implement the minimum
required to ensure that future kerberos-using clients won't break
unexpectedly on upgrade to 4.1.

--b.

2011-10-28 00:23:21

by Boaz Harrosh

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On 10/27/2011 01:34 PM, J. Bruce Fields wrote:
> On Tue, Oct 25, 2011 at 08:16:08PM -0700, Boaz Harrosh wrote:
>> On 10/25/2011 05:19 AM, J. Bruce Fields wrote:
>>> Boaz Harrosh (1):
>>> nfsd4: fix failure to end nfsd4 grace period
>>>
>>
>> Bruce hi.
>>
>> As I recall this failure I have experienced in early stages of the
>> 3.1-rcX series. I admit that I always run with Benny's pnfs tree,
>> but I do not think it is patches that Benny added from the 3.2 set
>> I think it is breakage introduced by the 3.1 merge window.
>>
>> Therefor I think that this patch must be sent to Stable@ otherwise
>> surly 3.1 Kernel users are to hit it. (You can escapee it)
>
> We need to test it, but I think you're probably correct,
> 6577aac01f00636c16cd583c30bd4dedf18475d5 "nfsd4: fix failure to end
> nfsd4 grace period" should have been tagged for stable. Does that also
> require any previous patches? (E.g.
> 48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7 "nfsd4: simplify recovery dir
> setting"?)
>

right, and all for that dmesg print. But yes!

Well the 3.1 Kernel without them gets stuck. I guess you need to experiment
with what would be the minimal patchset needed for 3.1 .

Tell me if you need help testing that the damage has gone. But basically I have
a script (4 years old) that does:
server: service nfs stop/start
client: mount
back to back, The client gets stuck forever.

> --b.

Thanks
Boaz

2011-10-26 03:31:47

by Boaz Harrosh

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On 10/25/2011 08:16 PM, Boaz Harrosh wrote:
> On 10/25/2011 05:19 AM, J. Bruce Fields wrote:
>> Changes for nfsd (and also a few file-locking changes) are available in
>> the git repository at:
>>
>> git://linux-nfs.org/~bfields/linux.git for-3.2
>>
>> This is the same tree I've always used--I assume you believe it's mine
>> without a need for signing anything. Feel free to tell me otherwise.
>>
>
> <Snip>
>
>>
>> Boaz Harrosh (1):
>> nfsd4: fix failure to end nfsd4 grace period
>>
>
> Bruce hi.
>
> As I recall this failure I have experienced in early stages of the
> 3.1-rcX series. I admit that I always run with Benny's pnfs tree,
> but I do not think it is patches that Benny added from the 3.2 set
> I think it is breakage introduced by the 3.1 merge window.
>
> Therefor I think that this patch must be sent to Stable@ otherwise
> surly 3.1 Kernel users are to hit it. (You can escapee it)
>

Rrrr spell checker for you. Above should be (You can't escape it)

Boaz
> Though it has been pulled to Linus tree, without the CC: stable@kernel
> I think you can just send the patch to <[email protected]> and it
> will be included.
>
> (What other 3.1 bug fixes are missing)
>
> Thanks
> Boaz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2011-10-28 14:42:55

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On Thu, Oct 27, 2011 at 05:23:04PM -0700, Boaz Harrosh wrote:
> On 10/27/2011 01:34 PM, J. Bruce Fields wrote:
> > We need to test it, but I think you're probably correct,
> > 6577aac01f00636c16cd583c30bd4dedf18475d5 "nfsd4: fix failure to end
> > nfsd4 grace period" should have been tagged for stable. Does that also
> > require any previous patches? (E.g.
> > 48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7 "nfsd4: simplify recovery dir
> > setting"?)
> >
>
> right, and all for that dmesg print. But yes!
>
> Well the 3.1 Kernel without them gets stuck. I guess you need to experiment
> with what would be the minimal patchset needed for 3.1 .
>
> Tell me if you need help testing that the damage has gone. But basically I have
> a script (4 years old) that does:
> server: service nfs stop/start
> client: mount
> back to back, The client gets stuck forever.

You and I are pretty sure those patches are needed, and fix the problem.
But it would probably be a good idea, to make sure.

Would you mind passing both along to [email protected]? Cc: me and I'll
ack them.

--b.

2011-10-27 20:34:15

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On Tue, Oct 25, 2011 at 08:16:08PM -0700, Boaz Harrosh wrote:
> On 10/25/2011 05:19 AM, J. Bruce Fields wrote:
> > Boaz Harrosh (1):
> > nfsd4: fix failure to end nfsd4 grace period
> >
>
> Bruce hi.
>
> As I recall this failure I have experienced in early stages of the
> 3.1-rcX series. I admit that I always run with Benny's pnfs tree,
> but I do not think it is patches that Benny added from the 3.2 set
> I think it is breakage introduced by the 3.1 merge window.
>
> Therefor I think that this patch must be sent to Stable@ otherwise
> surly 3.1 Kernel users are to hit it. (You can escapee it)

We need to test it, but I think you're probably correct,
6577aac01f00636c16cd583c30bd4dedf18475d5 "nfsd4: fix failure to end
nfsd4 grace period" should have been tagged for stable. Does that also
require any previous patches? (E.g.
48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7 "nfsd4: simplify recovery dir
setting"?)

--b.

2011-10-26 04:41:44

by Tom Tucker

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

Hi Bruce,

What does RDMA Non Support mean exactly? Maybe I could help if it's a
resource issue?

Thanks,
Tom

On 10/25/11 7:34 AM, J. Bruce Fields wrote:
> On Tue, Oct 25, 2011 at 08:19:24AM -0400, bfields wrote:
>> - Progress on the basic 4.1 todo's. Thanks in particular to Mi
>> Jinlong for (among other things) implementing the somewhat
>> tricky DRC limit checking.
>>
>> The 4.1 code is getting closer--it *might* be possible to
>> finish basic 4.1 early as 3.3, at which point we could start
>> on optional features (like pNFS). But that will depend on
>> people sending patches (and pynfs tests) for the remaining 4.1
>> todo's.
> I've been keeping the todo list up to date here:
>
> http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues
>
> Some of them I think are relatively straightforward--probably anyone
> with a little time could dive into them:
>
> - SEQ4_STATUS_RECALLABLE_STATE_REVOKED
>
> - backchannel attribute negotiation
>
> - ACL retention bits
>
> - Clarify RDMA non-support
>
> Slightly trickier or open-ended; we may need to talk over design first
> before diving in:
>
> - Make DESTROY_SESSION wait on in-progress requests
>
> - current stateid
>
> - Check 4.0/4.1 interactions
>
> - Callback failure handling
>
> - GSS
>
> The GSS piece is probably the one I'm most worried about. What we have
> seems sufficient for current clients, but I know it's incomplete, and
> I'm not entirely clear how much work it is to implement the minimum
> required to ensure that future kerberos-using clients won't break
> unexpectedly on upgrade to 4.1.
>
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2011-11-07 16:21:19

by Benny Halevy

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On 2011-11-04 18:50, Boaz Harrosh wrote:
> On 11/04/2011 09:04 AM, J. Bruce Fields wrote:
>> Boaz points out that I forgot to add stable cc's to two commits:
>>
>> 48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7 "nfsd4: simplify recovery dir setting"
>> 6577aac01f00636c16cd583c30bd4dedf18475d5 "nfsd4: fix failure to
>> end nfsd4 grace period"
>>
>> which fix a regression introduced by
>> ab1350b2b3c1dd2e465a6abdda608d8c44facfb8 "nfsd41: Deny new lock before
>> RECLAIM_COMPLETE done" (first included in v3.1-rc1).
>>
>> Included below for reference, but I assume simplest for you will be to
>> cherry-pick the above two sha1's.
>>
>> Thanks!
>>
>> --b.
>>
>
> Bruce hi, thanks
>
> Yesterday I hit this bug again. I rebased on Benny's tree which did not have them
> for some reason. It's hunting me. ;-)

That's odd. What tip did you use? These patches should be there:
8474c1e95786608a79849070b34f93924fcad087
530b7be2fbd79a8758230d53fc5f050d5c1561ba

Benny

>
> Just for Future reference, these patches were suppose to be submitted very early around
> rc3 when we found them. This is what the -rcX are for, to test and fix what was broken
> in the merge window.
>
> Please forgive me I should have followed it through, but was too busy and forgot.
>
> But in general you should always have an rc-fixes branch and bug fixes patches
> should go there and not into the linux-next branch. And yes both branches should
> be included in linux-next.
>
> This way there is nothing to forget
>
> Thanks
> Boaz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2011-11-04 16:57:17

by Boaz Harrosh

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On 11/04/2011 09:04 AM, J. Bruce Fields wrote:
> Boaz points out that I forgot to add stable cc's to two commits:
>
> 48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7 "nfsd4: simplify recovery dir setting"
> 6577aac01f00636c16cd583c30bd4dedf18475d5 "nfsd4: fix failure to
> end nfsd4 grace period"
>
> which fix a regression introduced by
> ab1350b2b3c1dd2e465a6abdda608d8c44facfb8 "nfsd41: Deny new lock before
> RECLAIM_COMPLETE done" (first included in v3.1-rc1).
>
> Included below for reference, but I assume simplest for you will be to
> cherry-pick the above two sha1's.
>
> Thanks!
>
> --b.
>

Bruce hi, thanks

Yesterday I hit this bug again. I rebased on Benny's tree which did not have them
for some reason. It's hunting me. ;-)

Just for Future reference, these patches were suppose to be submitted very early around
rc3 when we found them. This is what the -rcX are for, to test and fix what was broken
in the merge window.

Please forgive me I should have followed it through, but was too busy and forgot.

But in general you should always have an rc-fixes branch and bug fixes patches
should go there and not into the linux-next branch. And yes both branches should
be included in linux-next.

This way there is nothing to forget

Thanks
Boaz

2011-11-04 17:45:59

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

On Fri, Nov 04, 2011 at 09:50:16AM -0700, Boaz Harrosh wrote:
> On 11/04/2011 09:04 AM, J. Bruce Fields wrote:
> > Boaz points out that I forgot to add stable cc's to two commits:
> >
> > 48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7 "nfsd4: simplify recovery dir setting"
> > 6577aac01f00636c16cd583c30bd4dedf18475d5 "nfsd4: fix failure to
> > end nfsd4 grace period"
> >
> > which fix a regression introduced by
> > ab1350b2b3c1dd2e465a6abdda608d8c44facfb8 "nfsd41: Deny new lock before
> > RECLAIM_COMPLETE done" (first included in v3.1-rc1).
> >
> > Included below for reference, but I assume simplest for you will be to
> > cherry-pick the above two sha1's.
> >
> > Thanks!
> >
> > --b.
> >
>
> Bruce hi, thanks
>
> Yesterday I hit this bug again. I rebased on Benny's tree which did not have them
> for some reason. It's hunting me. ;-)
>
> Just for Future reference, these patches were suppose to be submitted very early around
> rc3 when we found them. This is what the -rcX are for, to test and fix what was broken
> in the merge window.
>
> Please forgive me I should have followed it through, but was too busy and forgot.
>
> But in general you should always have an rc-fixes branch and bug fixes patches
> should go there

More specifically (and especially later in the -rc's) Linus has ask that
we raise the bar somewhat above just "bug fixes"--but yes, this was a
regression with a pretty simple fix, so I should have gotten it in.

> and not into the linux-next branch. And yes both branches should
> be included in linux-next.

Agreed. Feel free to yell when you see something I missed. I'm feeling
spread a little thin....

--b.

2011-11-04 16:04:41

by J. Bruce Fields

[permalink] [raw]
Subject: Re: nfsd (and lock) changes for 3.2

Boaz points out that I forgot to add stable cc's to two commits:

48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7 "nfsd4: simplify recovery dir setting"
6577aac01f00636c16cd583c30bd4dedf18475d5 "nfsd4: fix failure to
end nfsd4 grace period"

which fix a regression introduced by
ab1350b2b3c1dd2e465a6abdda608d8c44facfb8 "nfsd41: Deny new lock before
RECLAIM_COMPLETE done" (first included in v3.1-rc1).

Included below for reference, but I assume simplest for you will be to
cherry-pick the above two sha1's.

Thanks!

--b.

commit 6577aac01f00636c16cd583c30bd4dedf18475d5
Author: Boaz Harrosh <[email protected]>
Date: Fri Aug 12 17:30:12 2011 -0700

nfsd4: fix failure to end nfsd4 grace period

Even if we fail to write a recovery record, we should still mark the
client as having acquired its first state. Otherwise we leave 4.1
clients with indefinite ERR_GRACE returns.

However, an inability to write stable storage records may cause failures
of reboot recovery, and the problem should still be brought to the
server administrator's attention.

So, make sure the error is logged.

These errors shouldn't normally be triggered on a corectly functioning
server--this isn't a case where a misconfigured client could spam the
logs.

Signed-off-by: Boaz Harrosh <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index c346661..493851b 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -130,6 +130,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
if (!rec_file || clp->cl_firststate)
return 0;

+ clp->cl_firststate = 1;
status = nfs4_save_creds(&original_cred);
if (status < 0)
return status;
@@ -144,10 +145,8 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
goto out_unlock;
}
status = -EEXIST;
- if (dentry->d_inode) {
- dprintk("NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS\n");
+ if (dentry->d_inode)
goto out_put;
- }
status = mnt_want_write(rec_file->f_path.mnt);
if (status)
goto out_put;
@@ -157,12 +156,14 @@ out_put:
dput(dentry);
out_unlock:
mutex_unlock(&dir->d_inode->i_mutex);
- if (status == 0) {
- clp->cl_firststate = 1;
+ if (status == 0)
vfs_fsync(rec_file, 0);
- }
+ else
+ printk(KERN_ERR "NFSD: failed to write recovery record"
+ " (err %d); please check that %s exists"
+ " and is writeable", status,
+ user_recovery_dirname);
nfs4_reset_creds(original_cred);
- dprintk("NFSD: nfsd4_create_clid_dir returns %d\n", status);
return status;
}


commit 48483bf23a568f3ef4cc7ad2c8f1a082f10ad0e7
Author: J. Bruce Fields <[email protected]>
Date: Fri Aug 26 20:40:28 2011 -0400

nfsd4: simplify recovery dir setting

Move around some of this code, simplify a bit.

Reviewed-by: Boaz Harrosh <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 29d77f6..c346661 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -45,6 +45,7 @@

/* Globals */
static struct file *rec_file;
+static char user_recovery_dirname[PATH_MAX] = "/var/lib/nfs/v4recovery";

static int
nfs4_save_creds(const struct cred **original_creds)
@@ -354,13 +355,13 @@ nfsd4_recdir_load(void) {
*/

void
-nfsd4_init_recdir(char *rec_dirname)
+nfsd4_init_recdir()
{
const struct cred *original_cred;
int status;

printk("NFSD: Using %s as the NFSv4 state recovery directory\n",
- rec_dirname);
+ user_recovery_dirname);

BUG_ON(rec_file);

@@ -372,10 +373,10 @@ nfsd4_init_recdir(char *rec_dirname)
return;
}

- rec_file = filp_open(rec_dirname, O_RDONLY | O_DIRECTORY, 0);
+ rec_file = filp_open(user_recovery_dirname, O_RDONLY | O_DIRECTORY, 0);
if (IS_ERR(rec_file)) {
printk("NFSD: unable to find recovery directory %s\n",
- rec_dirname);
+ user_recovery_dirname);
rec_file = NULL;
}

@@ -390,3 +391,30 @@ nfsd4_shutdown_recdir(void)
fput(rec_file);
rec_file = NULL;
}
+
+/*
+ * Change the NFSv4 recovery directory to recdir.
+ */
+int
+nfs4_reset_recoverydir(char *recdir)
+{
+ int status;
+ struct path path;
+
+ status = kern_path(recdir, LOOKUP_FOLLOW, &path);
+ if (status)
+ return status;
+ status = -ENOTDIR;
+ if (S_ISDIR(path.dentry->d_inode->i_mode)) {
+ strcpy(user_recovery_dirname, recdir);
+ status = 0;
+ }
+ path_put(&path);
+ return status;
+}
+
+char *
+nfs4_recoverydir(void)
+{
+ return user_recovery_dirname;
+}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index aa0a36e..36d0beb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -64,8 +64,6 @@ static struct nfs4_stateid * find_stateid(stateid_t *stid, int flags);
static struct nfs4_stateid * search_for_stateid(stateid_t *stid);
static struct nfs4_delegation * search_for_delegation(stateid_t *stid);
static struct nfs4_delegation * find_delegation_stateid(struct inode *ino, stateid_t *stid);
-static char user_recovery_dirname[PATH_MAX] = "/var/lib/nfs/v4recovery";
-static void nfs4_set_recdir(char *recdir);
static int check_for_locks(struct nfs4_file *filp, struct nfs4_stateowner *lowner);

/* Locking: */
@@ -4523,7 +4521,7 @@ nfsd4_load_reboot_recovery_data(void)
int status;

nfs4_lock_state();
- nfsd4_init_recdir(user_recovery_dirname);
+ nfsd4_init_recdir();
status = nfsd4_recdir_load();
nfs4_unlock_state();
if (status)
@@ -4632,40 +4630,3 @@ nfs4_state_shutdown(void)
nfs4_unlock_state();
nfsd4_destroy_callback_queue();
}
-
-/*
- * user_recovery_dirname is protected by the nfsd_mutex since it's only
- * accessed when nfsd is starting.
- */
-static void
-nfs4_set_recdir(char *recdir)
-{
- strcpy(user_recovery_dirname, recdir);
-}
-
-/*
- * Change the NFSv4 recovery directory to recdir.
- */
-int
-nfs4_reset_recoverydir(char *recdir)
-{
- int status;
- struct path path;
-
- status = kern_path(recdir, LOOKUP_FOLLOW, &path);
- if (status)
- return status;
- status = -ENOTDIR;
- if (S_ISDIR(path.dentry->d_inode->i_mode)) {
- nfs4_set_recdir(recdir);
- status = 0;
- }
- path_put(&path);
- return status;
-}
-
-char *
-nfs4_recoverydir(void)
-{
- return user_recovery_dirname;
-}
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 5cfebe5..12c1b1e 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -467,7 +467,7 @@ extern void nfsd4_destroy_callback_queue(void);
extern void nfsd4_shutdown_callback(struct nfs4_client *);
extern void nfs4_put_delegation(struct nfs4_delegation *dp);
extern __be32 nfs4_make_rec_clidname(char *clidname, struct xdr_netobj *clname);
-extern void nfsd4_init_recdir(char *recdir_name);
+extern void nfsd4_init_recdir(void);
extern int nfsd4_recdir_load(void);
extern void nfsd4_shutdown_recdir(void);
extern int nfs4_client_to_reclaim(const char *name);