use taskqueue_enqueue_fast when enqueue to taskqueue_fast

[sbruno]

From the manpage:

The function taskqueue_enqueue_fast() should be used in place of
taskqueue_enqueue() when the enqueuing must happen from a fast interrupt
handler.  This method uses spin locks to avoid the possibility of
sleeping in the fast interrupt context.

I'm unsure if this is needed as interrupts are disabled about 3 lines above via em_disable_intr().

In addition, if this is needed then the que_task should be done via taskqueue_enqueue_fast() as well.

em_disable_intr(adapter);
taskqueue_enqueue(adapter->tq, &adapter->que_task);

/* Link status change */
if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC)) {
        adapter->hw.mac.get_link_status = 1;
        taskqueue_enqueue(taskqueue_fast, &adapter->link_task);
}

[howard0su_gmail.com]

In D4930#104680, @sbruno wrote:

From the manpage:

The function taskqueue_enqueue_fast() should be used in place of
taskqueue_enqueue() when the enqueuing must happen from a fast interrupt
handler.  This method uses spin locks to avoid the possibility of
sleeping in the fast interrupt context.

I'm unsure if this is needed as interrupts are disabled about 3 lines above via em_disable_intr().

I think we need since there can be other consumer of this global queue.

In addition, if this is needed then the que_task should be done via taskqueue_enqueue_fast() as well.

This is different since adapter->tq is not a global queue.

em_disable_intr(adapter);
taskqueue_enqueue(adapter->tq, &adapter->que_task);

/* Link status change */
if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC)) {
        adapter->hw.mac.get_link_status = 1;
        taskqueue_enqueue(taskqueue_fast, &adapter->link_task);
}

This is what I read from man page:

To	use these queues, call taskqueue_enqueue() with	the value of the
global taskqueue variable for the queue you wish to use (taskqueue_swi,
taskqueue_swi_giant, or taskqueue_thread).	 Use taskqueue_enqueue_fast()
for the global taskqueue variable taskqueue_fast.

[erj]

Going by the man page, yeah, it looks like you should use taskqueue_enqueue_fast() instead of taskqueue_enqueue() if you're using the global taskqueue_fast.

Does this change fix any problems that you've noticed, or is this more of a correctness fix?

[howard0su_gmail.com]

In D4930#108388, @erj wrote:

Going by the man page, yeah, it looks like you should use taskqueue_enqueue_fast() instead of taskqueue_enqueue() if you're using the global taskqueue_fast.

Does this change fix any problems that you've noticed, or is this more of a correctness fix?

I noticed this when I was using cscope to read taskqueue related code. I don't even have a machine with e1000 network card. So this is purely correctness fix.

[sbruno]

John: Is there any reason to *not* do this?

[jhb]

taskqueue_enqueue_fast() isn't needed anymore, and the manpage probably needs to be updated. See the implementation of taskqueue_enqueue_fast():

/* NB: for backwards compatibility */
int
taskqueue_enqueue_fast(struct taskqueue *queue, struct task *task)
{
        return taskqueue_enqueue(queue, task);
}

This was reworked back in 7.0-CURRENT:

https://svnweb.freebsd.org/base/head/sys/kern/subr_taskqueue.c?revision=154167&view=markup

At this point taskqueue_enqueue_fast() should just be removed entirely (and removed from the manpage).

[howard0su_gmail.com]

In D4930#109015, @jhb wrote:

taskqueue_enqueue_fast() isn't needed anymore, and the manpage probably needs to be updated. See the implementation of taskqueue_enqueue_fast():

/* NB: for backwards compatibility */
int
taskqueue_enqueue_fast(struct taskqueue *queue, struct task *task)
{
        return taskqueue_enqueue(queue, task);
}

This was reworked back in 7.0-CURRENT:

https://svnweb.freebsd.org/base/head/sys/kern/subr_taskqueue.c?revision=154167&view=markup

At this point taskqueue_enqueue_fast() should just be removed entirely (and removed from the manpage).

Does that break KPI? I can propose the changes.

[howard0su_gmail.com]

I am wrong. the fix doesn't make sense.