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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey!

Sorry for the delay... Red Hat just rejiggered my
entire email world... fun fun! ;-)

On 5/27/21 12:47 PM, Olga Kornievskaia wrote:
On Thu, May 27, 2021 at 8:52 AM Steve Dickson <SteveD@xxxxxxxxxx> wrote:



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..

Hi Steve,

Can you please provide a bit more time for review to happen?
Fair enough... Scott a V3 version on last Thur.


Again... Nice work!!

Yes, nice work. But, I object to the current code that sets canceling
threads as default. This way the code hides the problems that occur
instead of forcing people to fix them.
Scott correct me if I'm wrong...

If the upcall is canceled (which is the default) the
upcall is failed causing the mount to fail and
a message is logged.

If the upcall is not canceled (using the -C flag)
the upcall continues to hang, but only on message
is logged about the hang... and the mount will
continue to hang.

See 'scan_active_thread_list()' the 'case EBUSY:' case.

So in both cases a the problem will be logged.

steved.




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux