I think that this variant of the subr_taskqueue_enqueue_timeout is fine in isolation, i.e. if we ignore the callers.

Did you inspected all callers of the function in the head ? In fact, r306320 is what I mean when we discussed the issue elsewere, and I thnk that despite this change, all callers should be fixed to do the similar treatment to the 'stopped' flag as the mentioned revision.

@kib
Did you inspected all callers of the function in the head ?

There is currently only one place in the kernel where the return value of this function is checked and that is:

sys/fs/autofs/autofs.c:		error = taskqueue_enqueue_timeout(taskqueue_thread,
sys/fs/autofs/autofs.c-		    &ar->ar_task, autofs_timeout * hz);
sys/fs/autofs/autofs.c-		if (error != 0) {
sys/fs/autofs/autofs.c:			AUTOFS_WARN("taskqueue_enqueue_timeout() failed "
sys/fs/autofs/autofs.c-			    "with error %d", error);
sys/fs/autofs/autofs.c-		}

From what I can see, this patch will not cause a problem, because the place where the task is residing is a new allocated memory block, so taskqueue_enqueue_timeout() will always return zero.

FYI: r306320 doesn't fully fix the problem. We also need the reliable taskqueue drain.

@kib
Do you think the setting of the DT_DRAIN_IN_PROGRESS flag should be in its own function?

--HPS

In D8012#166287, @hselasky wrote:

Do you think the setting of the DT_DRAIN_IN_PROGRESS flag should be in its own function?

Do you mean, should you create two helper inline functions to set and clear the flag ? I do not think it is important since right now the functions are single-use.

In D8012#166290, @kib wrote:

In D8012#166287, @hselasky wrote:

Do you think the setting of the DT_DRAIN_IN_PROGRESS flag should be in its own function?

Do you mean, should you create two helper inline functions to set and clear the flag ? I do not think it is important since right now the functions are single-use.

Yes, I was just wondering if that would be useful in the case where you want to do flush instead of drain, like inserting a barrier. Right now there is no need for such a function.

I'll wait for @trasz to respond and then I'll push.

Thank you!

@wblock
https://svnweb.freebsd.org/changeset/base/306634

In D8012#168259, @hselasky wrote:

@wblock
https://svnweb.freebsd.org/changeset/base/306634