[Nfs-ganesha-devel] AVL Tree Inline Coding

2017-03-10 Thread Frank Filz 
Daniel raised a good point about it being naughty that all our AVL trees
have an open coded lookup function.

On the other hand, there was originally a desire to have lookups code as
fast as possible, with an inline comparison function.

What if we did the following in avltree.h:

/*
 * 'pparent', 'unbalanced' and 'is_left' are only used for
 * insertions. Normally GCC will notice this and get rid of them for
 * lookups.
 */
static inline struct avltree_node *do_lookup(const struct avltree_node *key,
 const struct avltree *tree,
 struct avltree_node **pparent,
 struct avltree_node
**unbalanced,
 int *is_left,
 avltree_cmp_fn_t cmp_fn)
{
struct avltree_node *node = tree->root;
int res = 0;

*pparent = NULL;
*unbalanced = node;
*is_left = 0;

while (node) {
if (get_balance(node) != 0)
*unbalanced = node;

res = cmp_fn(node, key);
if (res == 0)
return node;
*pparent = node;
if ((*is_left = res > 0))
node = node->left;
else
node = node->right;
}
return NULL;
}

static inline
struct avltree_node *avltree_lookup(const struct avltree_node *key,
const struct avltree *tree,
avltree_cmp_fn_t cmp_fn)
{
struct avltree_node *parent, *unbalanced;
int is_left;

return do_lookup(key, tree, , , _left, cmp_fn);
}

With a given table defining an inline compare function, that allows it's
lookup calls to be completely inlined (with code that really is pretty
suitable for inlining).

If we wanted to make all uses of cmp_fn inline, avltree_insert could be
broken out as:

struct avltree_node *avltree_do_insert(struct avltree_node *node,
   struct avltree *tree,
   struct avltree_node *key,
   struct avltree_node *parent,
   struct avltree_node *unbalanced,
   int is_left)


static inline
struct avltree_node *avltree_insert(struct avltree_node *node,
struct avltree *tree,
avltree_cmp_fn_t cmp_fn)
{
struct avltree_node *key, *parent, *unbalanced;
int is_left;

key = do_lookup(node, tree, , , _left, cmp_fn);
if (key)
return key;

return avltree_do_insert(node, tree, key, parent, unbalanced,
is_left);
}

We then remove cmp_fn from struct avltree (it is also used in avltree_inf
and avltree_sup, but those functions will go away with the removal of the
old dirent cache).

Frank


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus


--
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] dispatch queues

2017-03-10 Thread William Allen Simpson 
On 3/9/17 10:22 AM, Matt Benjamin wrote:
>> From: "William Allen Simpson" 
>> After a somewhat loud discussion with Matt, we've agreed on a
>> different approach.  This will also be useful for fully async IO
>> that is planned for V2.6.
>
> Um, I don't think this statement represents either of the two internal 
> meetings we had accurately, but ok.
>
I think it does.  Please be more specific.  Or not, because these
offhand comments without substance are confusing to everybody on
this very public mailing list.


>> The sequential IO reports were from specific .
>
> For posterity, the feedback I've seen regarding sequential i/o was provided 
> by upstream folks on our regular concall. nobody uses the term "customer" on 
> this list, for obvious reasons.
>
The mailing list is the best place for these bug report discussions.
Also, IRC is totally worthless for recording discussions.

Because I've got an overlapping meeting on the first half hour, and
another on the second half hour, I don't attend many of the concalls
these days unless I've got something specific to report.

This "new" (since Thanksgiving 2015 anyway) time slot has never been
good for me.  I'd much prefer that it be moved to Wednesdays.  We don't
seem to get new dev releases until late Friday anyway.

I've always treated NFS as an IETF activity and follow IETF rules.
Unlike IEEE or CTIA (where it is against the rules), in IETF we always
talk about implementation, deployment, and customers quite specifically,
and confirm reported problems.  And success in fixing them.


> Well, as I think we've all agreed, nothing like this is going into 2.5.  
> Anything that DOES make it to the nfs-ganesha upstream is going to need to be 
> well motivated, well measured, and matured.
>
I thought you wanted my performance patches now.  Doing nothing at
this time is much easier

This particular patch was written for the second or third time back
2015, so it feels "mature" to me.  But then we are finally getting my
removal of the gsh_rpc.h [R|S]LOCK in this week, and I've got variants
dated Aug 3, 2015, and Sep 2, 2015, in my old-patches folder.

So, baby steps.  At least that gets rid of 3 layers of locks, so it
should be very helpful.

Since I've known about the queuing problem since the first time that
I'd looked at the code 3'ish years ago, and folks have customers who
are complaining, I'd thought this was an opportune time.

For RDMA, I simply bypassed it.

It is my considered opinion that fixing the rampant alloc/free, queuing,
and thread switches will do much more for TCP performance than async IO
that merely gives us a few percentage improvement at best.


>> To really do a good job, we need some kind of FSAL feedback API.
>> I'm going to ask the Gluster folks for some help on designing it, so
>> that we have a good use case and testing infrastructure.  But we'll
>> post the design iterations here in the same fashion as an IETF
>> Working Group, so that maybe we can get other FSAL feedback, too.
>>
>> Is anybody specifically interested in helping design the API?
>
> As the proposer of this idea, I'm interested in seeing experimental 
> prototypes that help us establish and refine something that works.  Let's 
> post running code, and then write specs.
>
API first to inform the design, then prototypes, then running code.

Oh, and for the record, you proposed that you would write up an API
over the Thanksgiving holidays, then the Christmas holidays.

Not seeing it, now I'm soliciting help from everybody.


> That said, upstream participation is welcome.
>
We're upstream here.


--
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] UDP duplicate cache in both Ganesha and ntirpc?

2017-03-10 Thread Matt Benjamin 
Hi,

- Original Message -
> From: "William Allen Simpson" 
> To: "Matt Benjamin" 
> Cc: d...@redhat.com, nfs-ganesha-devel@lists.sourceforge.net
> Sent: Friday, March 10, 2017 2:21:41 PM
> Subject: Re: [Nfs-ganesha-devel] UDP duplicate cache in both Ganesha and 
> ntirpc?
> 
> On 3/9/17 1:44 PM, Matt Benjamin wrote:
> > But, isn't su_cache...NULL?
> >
> Aha, I see that you are correct.  It is only set non-NULL in
> svc_dg_enablecache(), and that's never called.   Anywhere.
> 
> So we have this useless facility that I (and Malahal) have been
> trying to keep up-to-date with changes, and I've recently fixed
> the XXX !MT-SAFE (e89139b), and that's all for nought.
> 
> We don't cache TCP.  We don't cache RDMA.
> 
> This code is an anachronism, and needs to be purged with extreme
> prejudice.  It was badly written, and it's a shame to keep fixing.

Well, it is what it is.  Pretty durn old, and never used in nfs-ganesha, I 
don't believe.

> 
> If we get rid of it, we can use _ioq for output, and get rid of
> the extra locks.  Don't know how important to speed up UDP, but
> we could

This seems potentially a useful improvement, I wold say, so that provides some 
positive motivation for gc'ing the legacy cache stuff, I guess.

Matt

> 
> 

-- 
Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://www.redhat.com/en/technologies/storage

tel.  734-821-5101
fax.  734-769-8938
cel.  734-216-5309

--
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Permission denied error with Kerberos enabled

2017-03-10 Thread Satya Prakash GS 
On Sat, Mar 11, 2017 at 12:37 AM, William Allen Simpson
 wrote:
> I'm not familiar with this code, so not likely to be much help.
> Looks mostly written by Matt, but Malahal made the most recent
> changes in July 2016.
>
> On 3/10/17 9:35 AM, Satya Prakash GS wrote:
>>
>> Is this a possibility :
>>
>> Server first rejects a client op with CREDPROBLEM/REJECTEDCRED,
>> Client does an upcall and gssd initializes the context with the server.
>> However the server recycles it immediately before the operation was
>> retried (looks like there is a bug in the LRU implementation on
>> Ganesha. To make things worse, I enabled the server debugs and it
>> slowed down the client operations making the eviction of the entry
>> easier). This happens thrice failing the client op.
>>
> Problem is not obvious.
>
> axp->gen is initialized to zero with the rest of *axp -- mem_zalloc().
>
> gd->gen is initialized to zero by alloc_svc_rpc_gss_data().
>
> axp->gen is bumped by one (++) each time it is handled by LRU code in
> authgss_ctx_hash_get().
>

If a node gen isn't getting incremented it means that node is not
being looked up often.

> atomic_inc_uint32_t(>gen) is immediately after that.
>
> You think gd->gen also needs to be set to axp->gen in _set()?
>

> I'm not sure they are related.  There are many gd per axp, so
> axp->gen could be much higher than gd->gen.
>

>From authgss_ctx_gc_idle ->

if (abs(axp->gen - gd->gen) > __svc_params->gss.max_idle_gen) {
Remove the entry from the tree; //gd is no more in the cache after this
}

Translates to - gd wasn't looked up in quite sometime let's clean it up.

//gss.max_idle_gen -> by default set to 1024

If tree's gen is 5000 and a new node gets inserted into the tree, node
gen shouldn't start at 0 or it might pass the above condition in the
next authgss_ctx_gc_idle call.

> Both _get and _set are only called in svc_auth_gss.c _svcauth_gss().
>
> Admittedly, it is hard to track that there are 2 fields both called gen.
>
>> Thanks,
>> Satya.
>>
>> On Thu, Mar 9, 2017 at 8:07 PM, Satya Prakash GS
>>  wrote:
>>>
>>> Looks like the gen field in svc_rpc_gss_data is used to check the
>>> freshness of a context. However it is not initialized to axp->gen in
>>> authgss_ctx_hash_set.
>>> Will this not result in evicting the entries out early or am I missing
>>> something ?
>>>
>>> Thanks,
>>> Satya.
>>>
>>
>>
>> --
>> Announcing the Oxford Dictionaries API! The API offers world-renowned
>> dictionary content that is easy and intuitive to access. Sign up for an
>> account today to start using our lexical data to power your apps and
>> projects. Get started today and enter our developer competition.
>> http://sdm.link/oxford
>> ___
>> Nfs-ganesha-devel mailing list
>> Nfs-ganesha-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
>>
>

Thanks,
Satya.

--
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] Permission denied error with Kerberos enabled

2017-03-10 Thread Satya Prakash GS 
Is this a possibility :

Server first rejects a client op with CREDPROBLEM/REJECTEDCRED,
Client does an upcall and gssd initializes the context with the server.
However the server recycles it immediately before the operation was
retried (looks like there is a bug in the LRU implementation on
Ganesha. To make things worse, I enabled the server debugs and it
slowed down the client operations making the eviction of the entry
easier). This happens thrice failing the client op.

Thanks,
Satya.

On Thu, Mar 9, 2017 at 8:07 PM, Satya Prakash GS
 wrote:
> Looks like the gen field in svc_rpc_gss_data is used to check the
> freshness of a context. However it is not initialized to axp->gen in
> authgss_ctx_hash_set.
> Will this not result in evicting the entries out early or am I missing
> something ?
>
> Thanks,
> Satya.
>

--
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] Change in ffilz/nfs-ganesha[next]: FSAL_PROXY (support_ex without state) : pxy_commit2

2017-03-10 Thread GerritHub 
>From Patrice LUCAS :

Patrice LUCAS has uploaded this change for review. ( 
https://review.gerrithub.io/352204


Change subject: FSAL_PROXY (support_ex without state) : pxy_commit2
..

FSAL_PROXY (support_ex without state) : pxy_commit2

Change-Id: Ia89e0b74bcadc729a917dd16bb51ace6f463f61e
Signed-off-by: Patrice LUCAS 
---
M src/FSAL/FSAL_PROXY/fsal_nfsv4_macros.h
M src/FSAL/FSAL_PROXY/handle.c
2 files changed, 60 insertions(+), 0 deletions(-)



  git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha 
refs/changes/04/352204/1
-- 
To view, visit https://review.gerrithub.io/352204
To unsubscribe, visit https://review.gerrithub.io/settings

Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia89e0b74bcadc729a917dd16bb51ace6f463f61e
Gerrit-Change-Number: 352204
Gerrit-PatchSet: 1
Gerrit-Owner: Patrice LUCAS 
--
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel