[pnfs] [PATCH] pnfs: initialize ds_[rw]size only after thelayout driver has registered
Benny Halevy
bhalevy at panasas.com
Thu Jan 3 13:16:43 EST 2008
On Jan. 03, 2008, 19:55 +0200, "Labiaga, Ricardo" <Ricardo.Labiaga at netapp.com> wrote:
> 2.6.24-rc6 doesn't trigger this problem because the client is not
> setting the SESSION4_BACK_CHAN bit during session creation, so the
> server doesn't attempt the callback probe. It will with the backchannel
> changes after I'm done addressing the style issues you pointed out ;-)
Right.
I just found my bug in patch bddf81fea858dbdfee4a4f3bda10f86bdfde65f1
Your original code called xprt_set_bound after the next: label
and I changed that.
The following patch fixes that:
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 9b35536..a3935ed 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2271,6 +2271,7 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
#if defined(CONFIG_NFSD_V4_1)
if (args->svsk) {
/* backchannel */
+ xprt_set_bound(xprt);
INIT_DELAYED_WORK(&transport->connect_worker,
bc_connect_worker);
xprt->bind_timeout = 0;
I'll submit and push the fix right away.
Sorry for the breakage...
Benny
>
> - ricardo
>
>
>> -----Original Message-----
>> From: Benny Halevy [mailto:bhalevy at panasas.com]
>> Sent: Thursday, January 03, 2008 12:45 AM
>> To: Muntz, Daniel; Sager, Mike
>> Cc: pnfs at linux-nfs.org; Labiaga, Ricardo
>> Subject: Re: [pnfs] [PATCH] pnfs: initialize ds_[rw]size only
>> after thelayout driver has registered
>>
>> On Jan. 03, 2008, 3:34 +0200, "Muntz, Daniel"
>> <Dan.Muntz at netapp.com> wrote:
>>> Two issues. One, a "bad" client shouldn't oops the 2.6.24 kernel no
>>> matter which draft they're each on, so something needs to be fixed.
>>> Two, the 2.6.18 client was working with the labiaga
>> 2.6.24rc2 tree which
>>> I thought was draft 13 compliant.
>> Agreed. I'm testing my changes with the 2.6.24-rc6 client
>> (with Andy's
>> changes and myself's) I'll try to reproduce this with the
>> 2.6.18.3 client
>> as well.
>>
>> Benny
>>
>>> -----Original Message-----
>>> From: Dean Hildebrand [mailto:seattleplus at gmail.com]
>>> Sent: Wednesday, January 02, 2008 3:27 PM
>>> To: Sager, Mike
>>> Cc: pnfs at linux-nfs.org; Labiaga, Ricardo
>>> Subject: Re: [pnfs] [PATCH] pnfs: initialize ds_[rw]size only after
>>> thelayout driver has registered
>>>
>>> Thanks Ricardo. I updated the wiki with that information.
>>>
>>> Who will be updating this tree with the patches?
>>>
>>> Sager, Mike wrote:
>>>> As a sidenote, it appears this 2.6.24 repository isn't
>> compatible with
>>>> what I believe is the latest 2.6.18 client. When I try
>> it, the server
>>>> oops due to a NULL pointer dereference with the following trace:
>>>>
>>> The 2.6.18 code base did not make the jump to draft-13 after the
>>> bakeathon. I believe Ricardo applied our bakeathon draft
>> 13 patches to
>>> the 2.6.24 git tree, which I think Benny used to create his
>> tree (via
>>> Andy). I'm not sure of your specific error, but since it
>> is in decode
>>> compound, I bet that is the problem. The summary is that the 2.6.18
>>> code base is only compatible with itself.
>>> Dean
>>>> Call Trace:
>>>> [<c0369b48>] call_bind+0x65/0x6b
>>>> [<c0369bf9>] call_reserve+0x3c/0x65
>>>> [<c036fe9d>] __rpc_execute+0x6f/0x214 [<c036f768>]
>>>> rpc_set_active+0x3a/0x5c [<c036a632>] rpc_do_run_task+0x76/0x8f
>>>> [<c036b485>] rpc_call_async+0x1b/0x2d [<c01ffc60>]
>>>> nfsd41_probe_callback+0x1b8/0x213 [<c01282ed>]
>>>> local_bh_enable_ip+0x40/0x56 [<c01fbccc>]
>>>> nfsd4_create_session+0x132/0x340 [<c01fbb9a>]
>>>> nfsd4_create_session+0x0/0x340 [<c01ee543>]
>>>> nfsd4_proc_compound+0x272/0x3e9 [<c01f3cce>]
>>>> nfs4svc_decode_compoundargs+0x0/0x5c
>>>> [<c01e0ad2>] nfsd_dispatch+0xd3/0x1a0 [<c03759ce>]
>>>> svcauth_unix_set_client+0x176/0x1ac
>>>> [<c0372427>] svc_process+0x3be/0x66f
>>>> [<c0375249>] svc_recv+0x341/0x3b4
>>>> [<c01e1031>] nfsd+0x181/0x290
>>>> [<c01e0eb0>] nfsd+0x0/0x290
>>>> [<c0104b3b>] kernel_thread_helper+0x7/0x10
>>>>
>>>>
>>>> What client are people using to test pnfs? Last I heard, the
>>>> 2.6.24-based client doesn't yet support pnfs.
>>>>
>>>> Mike
>>>>
>>>> -----Original Message-----
>>>> From: Labiaga, Ricardo
>>>> Sent: Wednesday, January 02, 2008 2:35 PM
>>>> To: Dean Hildebrand; Benny Halevy
>>>> Cc: pnfs at linux-nfs.org
>>>> Subject: Re: [pnfs] [PATCH] pnfs: initialize ds_[rw]size
>> only after
>>>> thelayout driver has registered
>>>>
>>>>
>>>> Hi Dean,
>>>>
>>>> As I understand it, Benny is maintaining the new "golden"
>> repository
>>> at:
>>>> git://linux-nfs.org/~bhalevy/linux-pnfs.git
>>>>
>>>> I have a clone and track the following branches: nfs41
>> and pnfs. I
>>>> see there are other branches. Private to Benny's work?
>>>>
>>>> At least I am working under that understanding based on
>> our last pNFS
>>>> concall.
>>>>
>>>> - ricardo
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Dean Hildebrand [mailto:seattleplus at gmail.com]
>>>>> Sent: Wednesday, January 02, 2008 1:33 PM
>>>>> To: Benny Halevy
>>>>> Cc: pnfs at linux-nfs.org
>>>>> Subject: Re: [pnfs] [PATCH] pnfs: initialize ds_[rw]size
>> only after
>>>>> the layout driver has registered
>>>>>
>>>>> I'm confused with the large numbers of git repositories
>> that we now
>>>>> have. My understanding of the process we were using (most
>>>>> recently) for
>>>>> 2.6.18 was
>>>>> 1. Make change in local repository
>>>>> 2. "git send-email --to pnfs at linux-nfs.org ..." your
>> changes 3. Andy
>>>>> or Bruce would apply the patches to the pNFS repository
>> (or do a git
>>>>> pull I guess)
>>>>>
>>>>> If this is still the process, what is the git repository I should
>>>>> pull from for step 1 and then have andy/bruce push to for
>> step 3? Is
>>>>> it Benny's? Benny, are the patches you sending out in your local
>>>>> repository or the one you created at citi? Once all of
>> the patches
>>>>> are approved, to which git tree will they be applied?
>>>>>
>>>>> If someone knows, can they update the wiki?
>>>>> Dean
>>>>>
>>>>> Benny Halevy wrote:
>>>>>
>>>>>> Currently, nfs_server_set_fsinfo, which sets the DS I/O
>>>>>>
>>>>> sizes, is being called
>>>>>
>>>>>> before pnfs has been initialized.
>>>>>>
>>>>>> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
>>>>>> ---
>>>>>> fs/nfs/client.c | 15 ---------------
>>>>>> fs/nfs/pnfs.c | 22 ++++++++++++++++++++++
>>>>>> fs/nfs/pnfs.h | 1 +
>>>>>> fs/nfs/super.c | 1 +
>>>>>> 4 files changed, 24 insertions(+), 15 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c index
>>>>>> f96aa40..6db3b83 100644
>>>>>> --- a/fs/nfs/client.c
>>>>>> +++ b/fs/nfs/client.c
>>>>>> @@ -620,9 +620,6 @@ error:
>>>>>> static void nfs_server_set_fsinfo(struct nfs_server
>>>>>>
>>>>> *server, struct nfs_fsinfo *fsinfo)
>>>>>
>>>>>> {
>>>>>> unsigned long max_rpc_payload;
>>>>>> -#ifdef CONFIG_PNFS
>>>>>> - unsigned long dssize;
>>>>>> -#endif
>>>>>>
>>>>>> /* Work out a lot of parameters */
>>>>>> if (server->rsize == 0)
>>>>>> @@ -652,18 +649,6 @@ static void
>>>>>>
>>>>> nfs_server_set_fsinfo(struct nfs_server *server, struct
>> nfs_fsinfo *
>>>>>
>>>>>> #ifdef CONFIG_PNFS
>>>>>> /* Save the layout type for use during init of
>> layout driver */
>>>>>> server->pnfs_fs_ltype = fsinfo->layoutclass;
>>>>>> -
>>>>>> - /* Set buffer size for data servers */
>>>>>> - dssize = pnfs_getiosize(server);
>>>>>> - if (dssize > 0) {
>>>>>> - server->ds_rsize = server->ds_wsize =
>>>>>>
>>>>> nfs_block_size(dssize, NULL);
>>>>>
>>>>>> - server->ds_rpages = server->ds_wpages =
>>>>>>
>>>>> (server->ds_rsize + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
>>>>>
>>>>>> - } else {
>>>>>> - server->ds_wsize = server->wsize;
>>>>>> - server->ds_rsize = server->rsize;
>>>>>> - server->ds_rpages = server->rpages;
>>>>>> - server->ds_wpages = server->wpages;
>>>>>> - }
>>>>>> #endif /* CONFIG_PNFS */
>>>>>>
>>>>>> server->wtmult = nfs_block_bits(fsinfo->wtmult,
>> NULL); diff
>>>>>>
>>>> --git
>>>>
>>>>>> a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index ca99c44..d49f9ce 100644
>>>>>> --- a/fs/nfs/pnfs.c
>>>>>> +++ b/fs/nfs/pnfs.c
>>>>>> @@ -46,6 +46,8 @@
>>>>>> #include <linux/pnfs_xdr.h>
>>>>>> #include <linux/nfs4_pnfs.h>
>>>>>>
>>>>>> +#include "internal.h"
>>>>>> +
>>>>>> #include "nfs4_fs.h"
>>>>>> #include "pnfs.h"
>>>>>>
>>>>>> @@ -859,6 +861,26 @@ pnfs_getiosize(struct nfs_server *server)
>>>>>> return ld->ld_policy_ops->get_blocksize(mounttype);
>>>>>> }
>>>>>>
>>>>>> +void
>>>>>> +pnfs_set_ds_iosize(struct nfs_server *server) {
>>>>>> + unsigned dssize = pnfs_getiosize(server);
>>>>>> +
>>>>>> + /* Set buffer size for data servers */
>>>>>> + if (dssize > 0) {
>>>>>> + server->ds_rsize = server->ds_wsize =
>>>>>> + nfs_block_size(dssize, NULL);
>>>>>> + server->ds_rpages = server->ds_wpages =
>>>>>> + (server->ds_rsize +
>> PAGE_CACHE_SIZE - 1) >>
>>>>>> + PAGE_CACHE_SHIFT;
>>>>>> + } else {
>>>>>> + server->ds_wsize = server->wsize;
>>>>>> + server->ds_rsize = server->rsize;
>>>>>> + server->ds_rpages = server->rpages;
>>>>>> + server->ds_wpages = server->wpages;
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> /* Invoked by all non-NFSv4 I/O layout drivers to mark
>>>>>>
>>>>> pages for commit
>>>>>
>>>>>> */
>>>>>> static void
>>>>>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h index
>> 50e3797..bb7b091
>>>>>> 100644
>>>>>> --- a/fs/nfs/pnfs.h
>>>>>> +++ b/fs/nfs/pnfs.h
>>>>>> @@ -46,6 +46,7 @@ int pnfs_enabled_sb(struct nfs_server
>> *nfss); int
>>>>>>
>>>>
>>>>>> pnfs_use_nfsv4_wproto(struct inode *inode, ssize_t count); int
>>>>>> pnfs_use_nfsv4_rproto(struct inode *inode, ssize_t
>> count); unsigned
>>>>>>
>>>>
>>>>>> int pnfs_getiosize(struct nfs_server *server);
>>>>>> +void pnfs_set_ds_iosize(struct nfs_server *server);
>>>>>> int pnfs_commit(struct inode *inode, struct list_head
>>>>>>
>>>>> *head, int sync, struct nfs_write_data *data);
>>>>>
>>>>>> int pnfs_try_to_commit(struct inode *, struct
>>>>>>
>>>>> nfs_write_data *, struct list_head *, int);
>>>>>
>>>>>> int pnfs_wsize(struct inode *, unsigned int, struct
>>>>>>
>>>>> nfs_write_data *);
>>>>>
>>>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c index
>> b4aecb1..bb6ffa2
>>>>>> 100644
>>>>>> --- a/fs/nfs/super.c
>>>>>> +++ b/fs/nfs/super.c
>>>>>> @@ -1705,6 +1705,7 @@ int nfs4_init_pnfs(struct super_block
>>>>>>
>>>>> *sb, struct nfs_server *server,
>>>>>
>>>>>> switch (clp->cl_minorversion) {
>>>>>> case 1:
>>>>>> set_pnfs_layoutdriver(sb, fh,
>> server->pnfs_fs_ltype);
>>>>>> + pnfs_set_ds_iosize(server);
>>>>>> break;
>>>>>> case 0:
>>>>>> break;
>>>>>>
>>>>>>
>> _______________________________________________
>> pNFS mailing list
>> pNFS at linux-nfs.org
>> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>>
More information about the pNFS
mailing list