Re: [v6 PATCH 4/4] bnx2fc: Makefile, Kconfig changes and FCoE interfaces

[Mike Christie <michaelc@cs.wisc.edu>]

On 02/03/2011 09:35 PM, Bhanu Gollapudi wrote:

> +
> +static int bnx2fc_l2_rcv_thread(void *arg)
> +{
> +	struct fcoe_percpu_s *bg = arg;
> +	struct sk_buff *skb;
> +
> +	while (!kthread_should_stop()) {
> +		spin_lock_bh(&bg->fcoe_rx_list.lock);
> +		while ((skb = __skb_dequeue(&bg->fcoe_rx_list)) == NULL) {
> +			set_current_state(TASK_INTERRUPTIBLE);
> +			spin_unlock_bh(&bg->fcoe_rx_list.lock);
> +			schedule();
> +			set_current_state(TASK_RUNNING);
> +			if (kthread_should_stop())
> +				return 0;
> +			spin_lock_bh(&bg->fcoe_rx_list.lock);
> +		}
> +		spin_unlock_bh(&bg->fcoe_rx_list.lock);
> +		bnx2fc_recv_frame(skb);
> +	}
> +	return 0;
> +}
> +
> +

I am not sure this is correct. Do you want to set the state to 
interruptible before calling kthread_should_stop and before scheduling? 
If kthread_stop is called after the check for kthread_should_stop() and 
before setting the state to interruptible then will you have missed 
kthread_stop's wake_up and will you schedule and not wake up again?

Do you need something like:

set_current_state(TASK_INTERRUPTIBLE);
while (!kthread_should_stop()) {
	schedule();
	set_current_state(TASK_RUNNING);
	spin_lock_bh(&bg->fcoe_rx_list.lock);
	while ((skb = __skb_dequeue(&bg->fcoe_rx_list)) != NULL) {
		spin_unlock_bh(&bg->fcoe_rx_list.lock);
		bnx2fc_recv_frame(skb);
		spin_lock_bh(&bg->fcoe_rx_list.lock);
	}
	set_current_state(TASK_INTERRUPTIBLE);
}
set_current_state(TASK_RUNNING);