Re: [nfs-utils RFC PATCH 2/2] gssd: add timeout for upcall threads

[Steve Dickson <SteveD@RedHat.com>]

On 5/27/21 7:40 AM, Scott Mayhew wrote:
> On Wed, 26 May 2021, Steve Dickson wrote:
>> If people are going to used the -C flag they are saying they want
>> to ignore hung threads so I'm thinking with printerr(0) we would
>> be filling up their logs about messages they don't care about.
>> So I'm thinking we should change this to a printerr(1) 
> 
> Note that message could pop multiple times per thread even without the
> -C flag because cancellation isn't immediate (a thread needs to hit a
> cancellation point, which it won't actually do that until it comes back
> from wherever it's hanging).  My thinking was leaving it with
> printerr(0) would make it blatantly obvious when something was wrong and
> needed to be investigated.  I have no issue with changing it to
> printerr(1) though. 
It would... but I've craft the debugging for a single -v 
is errors only... Maybe I should mention that in the
man page... And looking at what you mention in the
man page for -C, it does say it will cause an error 
to be logged... So I guess it makes sense to leave
it as is.

> 
> Alternatively we could add another flag to struct upcall_thread_info to
> ensure that message only gets logged once per thread.
> 
I think it is good as is... 

>>
>> Overall I think the code is very well written with
>> one exception... The lack of comments. I think it
>> would be very useful to let the reader know what
>> you are doing and why.... But by no means is 
>> that a show stopper. Nice work!
> 
> I can go back and add some comments.
Well there aren't that many comments to 
begin with.... So you are just following 
the format... ;-) 

Don't worry about it... How I will finish my testing
today... and do the commit with what we got.. 

Again... Nice work!!

steved.