Search Linux Wireless

Re: RFC: wilc1000: refactor TX path to use sk_buff queue

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

 



On Wed, 2021-12-15 at 12:44 +0000, Ajay.Kathat@xxxxxxxxxxxxx wrote:
> On 12/12/21 02:02, David Mosberger-Tang wrote:
> > Notes:
> > 
> >   - Don't try to apply this patch as is.  There are two other small
> >     but unrelated changes that this patch below depends on.
> 
> Btw what are the other two changes required to make it work. I believe 
> the working flow shouldn't get impacted after the restructuring 
> especially when code is refactor(replaced) using the existing solution.

I'll start working on a patch series.  There are some before and some
after the sk_buff refactor, but those are all pretty straight-forward
in comparison.

> > 
> > Thoughts and/or suggestions?
> 
> Using sk_buff queue is a good idea but the patch has other changes also. 
> This patch should be splited into a few different logical related 
> changes. Using a single patch for all these changes may not be easy to 
> review and handle if there are any regressions.
> 
> One way to split the changes could be like:
> 
>   - sk_buff handling for mgmt/data and config frames
>   - WID(config packets) functions refactoring
>   - Functions refactoring
>   - Rename related changes

Yeah, the WID refactoring and structure renaming can be split off
easily, good suggestion.  Let me start working on that.

> <snip>
> 
> >                  (*ac)++;
> >          
> > +static int wilc_wlan_cfg_apply_wid(struct wilc_vif *vif, int start, u16 wid,
> > +                                  u8 *buffer, u32 buffer_size, int commit,
> > +                                  u32 drv_handler, bool set)
> >   {
> > -       u32 offset;
> >          int ret_size;
> >          struct wilc *wilc = vif->wilc;
> > 
> >          mutex_lock(&wilc->cfg_cmd_lock);
> > 
> > -       if (start)
> > -               wilc->cfg_frame_offset = 0;
> > -
> > -       offset = wilc->cfg_frame_offset;
> > -       ret_size = wilc_wlan_cfg_set_wid(wilc->cfg_frame.frame, offset,
> > -                                        wid, buffer, buffer_size);
> > -       offset += ret_size;
> > -       wilc->cfg_frame_offset = offset;
> > -
> > -       if (!commit) {
> > -               mutex_unlock(&wilc->cfg_cmd_lock);
> > -               return ret_size;
> > +       if (start) {
> > +               WARN_ON(wilc->cfg_skb);
> > +               wilc->cfg_skb = alloc_cfg_skb(vif);
> > +               if (!wilc->cfg_skb) {
> 
> 'cfg_cmd_lock' mutex unlock is missing.

Indeed, thanks for catching that!

  --david





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux