All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: Emil Goode <emilgoode@gmail.com>
Cc: Steve Glendinning <steve.glendinning@shawell.net>,
	Oliver Neukum <oneukum@suse.de>,
	"David S. Miller" <davem@davemloft.net>,
	Freddy Xin <freddy@asix.com.tw>,
	Eric Dumazet <edumazet@google.com>,
	Ming Lei <ming.lei@canonical.com>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
	Liu Junliang <liujunliang_ljl@163.com>,
	Octavian Purdila <octavian.purdila@intel.com>,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usbnet: remove generic hard_header_len check
Date: Thu, 13 Feb 2014 12:36:30 +0100	[thread overview]
Message-ID: <87y51f2qip.fsf@nemi.mork.no> (raw)
In-Reply-To: <20140213112016.GA4245@lianli> (Emil Goode's message of "Thu, 13 Feb 2014 12:20:16 +0100")

Emil Goode <emilgoode@gmail.com> writes:

> Yes I should have put a comment in the changelog about this. All skbs that
> are passed to rx_process have their state set to rx_cleanup and just because
> the skb was cloned doesn't mean that we should free the original in a
> different way. As it is I think we are acctually missing a call to
> usb_free_urb that is called on the common rx_cleanup path.

Yes, I wondered about that as well, thinking that this part actually
should be a separate patch for net+stable.

But then I wondered how we could possibly have had a bug like that
living here for so long, and the answer is we haven't.  You don't want
to free the URB.  It is already resubmitted with a different skb as its
buffer.  The call to usb_free_urb in the rx_cleanup path is there only
for the cases where the URB is not resubmitted. The rx_complete callback
is controlling this by updating entry->urb as appropriate.  So it will
always be NULL if rx_process is called, and the call to usb_free_urb has
no effect.

Rearranging this code to always take the same cleanup path is still a
very nice cleanup IMHO.


Bjørn

  reply	other threads:[~2014-02-13 11:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-13  0:03 [PATCH] usbnet: remove generic hard_header_len check Emil Goode
2014-02-13  9:05 ` Bjørn Mork
2014-02-13 11:20   ` Emil Goode
2014-02-13 11:36     ` Bjørn Mork [this message]
2014-02-13 11:49     ` David Laight
2014-02-13 11:49       ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y51f2qip.fsf@nemi.mork.no \
    --to=bjorn@mork.no \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=emilgoode@gmail.com \
    --cc=freddy@asix.com.tw \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=liujunliang_ljl@163.com \
    --cc=ming.lei@canonical.com \
    --cc=netdev@vger.kernel.org \
    --cc=octavian.purdila@intel.com \
    --cc=oneukum@suse.de \
    --cc=paul.gortmaker@windriver.com \
    --cc=steve.glendinning@shawell.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.