Hi Kalle,

> -----Original Message-----
> From: Kalle Valo [mailto:kv...@codeaurora.org]
> Sent: Tuesday, March 03, 2015 6:08 PM
> To: Avinash Patil
> Cc: linux-wireless@vger.kernel.org; Amitkumar Karwar; Cathy Luo; Zhaoyang Liu;
> Shengzhen Li; Marc Yang
> Subject: Re: [PATCH 1/4] mwifiex: avoid queue_work while work is ongoing
> 
> Avinash Patil <pat...@marvell.com> writes:
> 
> > From: Marc Yang <yangy...@marvell.com>
> >
> > Current code does not check whether main_work_queue or rx_work_queue
> > is running when preparing to do queue_work, this code fix add check
> > before calling queue_work, reducing unnecessary queue_work switch.
> >
> > This change instead sets more_task flag to ensure we run main_process
> > superloop once again.
> >
> > Signed-off-by: Marc Yang <yangy...@marvell.com>
> > Signed-off-by: Zhaoyang Liu <li...@marvell.com>
> > Signed-off-by: Shengzhen Li <s...@marvell.com>
> > Signed-off-by: Cathy Luo <c...@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akar...@marvell.com>
> > Signed-off-by: Avinash Patil <pat...@marvell.com>
> 
> Really, it took six persons to write this patch? Or do you just dump the same
> names to each patch?

This should have been "Reviewed-by". Apologies.
 

> > +void mwifiex_queue_main_work(struct mwifiex_adapter *adapter) {
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(&adapter->main_proc_lock, flags);
> > +   if (adapter->mwifiex_processing) {
> > +           adapter->more_task_flag = true;
> > +           spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> > +   } else {
> > +           spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> > +           queue_work(adapter->workqueue, &adapter->main_work);
> > +   }
> > +}
> > +EXPORT_SYMBOL_GPL(mwifiex_queue_main_work);
> > +
> > +static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter) {
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(&adapter->rx_proc_lock, flags);
> > +   if (adapter->rx_processing) {
> > +           spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> > +   } else {
> > +           spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
> > +           queue_work(adapter->rx_workqueue, &adapter->rx_work);
> > +   }
> > +}
> 
> I can apply this patch, but to me this looks like a horrible hack.

We are here trying to avoid requeueing another work when work item is being 
executed. We set more_task flag to true if work item is being executed and 
mwifiex_main_process would execute superloop once again if more_task it set. 
work_pending() cannot be of much help here since we want to avoid queing only 
when work item is being executed. Could you please let us know if you think of 
any better way to handle this?
> --
> Kalle Valo

-Avinash
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to