Re: Problem with dev_kfree_skb_any() in 2.6.0

From: Jeff Garzik (jgarzik_at_pobox.com)
Date: 01/02/04

  • Next message: Paul Jackson: "Re: [PATCH] disable gcc warnings of sign/unsigned comparison" 
    Date:	Thu, 1 Jan 2004 21:58:07 -0500
To: "David S. Miller" <davem@redhat.com>

    On Thu, Jan 01, 2004 at 12:42:18PM -0800, David S. Miller wrote:
    > On Tue, 30 Dec 2003 12:43:21 -0500
    > Jeff Garzik <jgarzik@pobox.com> wrote:
    >
    > > Luckily, I feel there is an easy solution, as shown in the attached
    > > patch. We _already_ queue skbs in dev_kfree_skb_irq(). Therefore,
    > > dev_kfree_skb_any() can simply use precisely that same solution. The
    > > raise-softirq code will immediately proceed to action if we are not in
    > > hard IRQ context, otherwise it will follow the expected path.
    >
    > Ok, this is reasonable and works.
    >
    > Though, is there any particular reason you don't like adding a
    > "|| irqs_disabled()" check to the if statement instead?
    > I prefer that solution better actually.

    Yep, in fact when I wrote the above message, I came across a couple when I
    was pondering...
    * the destructor runs in a more predictable context.
    * given the problem that started this thread, the 'if' test is a
      potentially problematic area. Why not eliminate all possibility that
      this problem will occur again?

    The only counter argument to this -- to which I have no data to answer --
    is that there may be advantage to calling __kfree_skb immediately
    instead of deferring it slightly. I didn't think that disadvantage
    outweighted the above, but who knows... I can possibly be convinced
    otherwise. (and "otherwise" would be using || irqs_disabled())

    For the users who don't know/don't care about their context, it just
    seemed to me that they were not a hot path like users of dev_kfree_skb()
    and dev_kfree_skb_irq() [unconditional] are...

            Jeff

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  • Next message: Paul Jackson: "Re: [PATCH] disable gcc warnings of sign/unsigned comparison"

    Relevant Pages

    • Re: [pm] fix oops after saving image
      ... The patch is simply a band-aid, ... seeing that line solely in the context on the rest of the source ... In the pmdisk code, I've statically declared the ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [pm] fix oops after saving image
      ... > context of the email. ... > the reason for the patch, if they manipulated the BK tools correctly. ... When do you have a heart between your knees? ... To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ...
      (Linux-Kernel)
    • Re: file as a directory
      ... depends upon the context in the user-space ... /* check if the archive is a path component or if last ... No more worries! ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: file as a directory
      ... >depends upon the context in the user-space ... >and this context is reflected in the kernel space in the flags of the ... There is the O_DIRECTORY user-space flag, but which only says "it must be a ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: Add a context menu item to Windows Explorer (Win98) with VB6?
      ... clicked the context menu... ... Explorer started a new instance for each file and I got ... There isn't an easy solution, ... tired old brain and re-reading stuff like IShellExtInit, IContextMenu, ...
      (microsoft.public.vb.general.discussion)