* [PATCH net-next-2.6] net: skb_frag_t can be smaller on small arches
From: Eric Dumazet @ 2010-09-23 15:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
David
I remember you reacted on my three years old comment about
commit a309bb072b96bfe (Page offsets and lengths need to be __u32.)
http://www.mail-archive.com/netdev@vger.kernel.org/msg43999.html
Could we reconsider the thing now and allow to shrink
sizeof(skb_shared_info) from 0x104 to 0xbc, considering nothing happened
about scatterlist work ?
This saves 128 bytes on i386 because of alignments, and can be reverted
pretty fast if needed.
Thanks !
[PATCH net-next-2.6] net: skb_frag_t can be smaller on small arches
On 32bit arches, if PAGE_SIZE is smaller than 65536, we can use 16bit
offset and size fields. This patch saves 72 bytes per skb on i386, or
128 bytes after rounding.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/skbuff.h | 5 +++++
1 files changed, 5 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9e8085a..9a7ea70 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -129,8 +129,13 @@ typedef struct skb_frag_struct skb_frag_t;
struct skb_frag_struct {
struct page *page;
+#if (BITS_PER_LONG > 32) || (PAGE_SIZE >= 65536)
__u32 page_offset;
__u32 size;
+#else
+ __u16 page_offset;
+ __u16 size;
+#endif
};
#define HAVE_HW_TIME_STAMP
^ permalink raw reply related
* Re: [RFC PATCH] dont create cached routes from ARP requests
From: Eric Dumazet @ 2010-09-23 15:05 UTC (permalink / raw)
To: Ulrich Weber; +Cc: David Miller, netdev
In-Reply-To: <20100923144708.GA8037@babylon>
Le jeudi 23 septembre 2010 à 16:47 +0200, Ulrich Weber a écrit :
> On Wed, Sep 22, 2010 at 08:34:42PM -0700, David Miller wrote:
> > From: Ulrich Weber <uweber@astaro.com>
> > Date: Wed, 22 Sep 2010 18:22:09 +0200
> >
> > > Background: At home I have two Internet connections, DSL and Cable.
> > > DSL is the primary uplink while Cable is the secondary.
> > > My Cable ISP is flooding me with ARP request from 10.0.0.0/8,
> > > which creates routes via the primary uplink.
> > > There are thousands of cached routes and after some time
> > > I get "Neighbour table overflow" messages.
> >
> > If you get neighbour table overflows, something is holding a reference
> > to the routing cache entry and/or the neighbour entries those routing
> > cache entries are attached to.
> >
> > If these really are transient entries, they should be trivially
> > garbage collected and not cause any problems at all.
> rt_garbage_collect is not called within rt_intern_hash,
> because the call is done within softirq context.
>
> Forcing the call of rt_garbage_collect didn't help either,
> there are no routes freed afterwards...
>
Please give us more information.
grep . /proc/sys/net/ipv4/route/*
rtstat -c10 -i1
Thanks
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: Eric W. Biederman @ 2010-09-23 15:00 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: hadi, linux-kernel, Linux Containers, netdev, netfilter-devel,
linux-fsdevel, Daniel Lezcano, Linus Torvalds, Michael Kerrisk,
Ulrich Drepper, Al Viro, David Miller, Serge E. Hallyn,
Ben Greear, Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
Jan Engelhardt, Patrick McHardy
In-Reply-To: <4C9B495D.70200@parallels.com>
Pavel Emelyanov <xemul@parallels.com> writes:
> On 09/23/2010 04:11 PM, jamal wrote:
>> On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
>>
>>> Why does it matter? You told, that the usage scenario was to
>>> add routes to container. If I do 2 syscalls instead of 1, is
>>> it THAT worse?
>>>
>>
>> Anything to do with socket IO that requires namespace awareness
>> applies for usage; it could be tcp/udp/etc socket. If it doesnt
>> make any difference performance wise using one scheme vs other
>> to write/read heavy messages then i dont see an issue and socketat
>> is redundant.
>
> That's what my point is about - unless we know why would we need it
> we don't need it.
>
> Eric, please clarify, what is the need in creating a socket in foreign
> net namespace?
Strictly speaking with setns() you can implement this functionality
with setns(). aka
int socketat(int nsfd, int domain, int type, int protocol)
{
int sk;
setns(0, nsfd);
sk = socket(domain, type, protocol);
setns(0, default_nsfd);
return sk;
}
The major difference is that socketat in userspace suffers
from races, with signals etc.
The use case are applications are the handful of networking applications
that find that it makes sense to listen to sockets from multiple network
namespaces at once. Say a home machine that has a vpn into your office
network and the vpn into the office network runs in a different network
namespace so you don't have to worry about address conflicts between
the two networks, the chance of accidentally bridging between them,
and so you can use different dns resolvers for the different networks.
In that scenario it would be nice if I could run some services on both
networks. Starting two+ copies of the daemons just so the can have live
in all of the networks is ok, but in the fullness of time I expect that
there will be daemons that want to optimize things and have sockets in
all of the network namespaces you are connected to.
In a multiple network namespace aware application when it goes to open
a socket it will want to specify which network namespace the socket is
in. If it is a general listener it will probably listening to events
in /proc/mounts waiting for extra namespaces to be mounted under a
standard location say: /var/run/netns/<netnsname>/ns.
Once the application receives the event for a new network namespace
showing up it can will want to create a new socket listening for
connections in the new network namespace.
In that scenario none of those network namespaces are foreign, but one
network namespace will be the default and the rest will be non-default
network namespaces.
To support a multiple network namespace aware daemon I need to implement
sockeat() somewhere. So I figured I would see if anyone minded a
trivial in kernel race free implementation. To me it is a wart in the
API and I am busily removing warts in the API.
I don't know of any scenarios with other namespaces where there would be
applications that would be native in multiple namespaces. So I haven't
haven't done any work in that direction.
Eric
^ permalink raw reply
* Re: [PATCH 2/2 -next] r8169: use device model DMA API
From: Denis Kirjanov @ 2010-09-23 14:59 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Francois Romieu, netdev
In-Reply-To: <1285243291-4520-2-git-send-email-sgruszka@redhat.com>
On Thu, Sep 23, 2010 at 4:01 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Use DMA API as PCI equivalents will be deprecated. This change also
> allow to allocate with GFP_KERNEL where possible.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
dma_map_single and friend can fail. Thus, we should check for a return value.
Yes, this is not directly related to current patch, but it would be
great to fix this.
> drivers/net/r8169.c | 53 +++++++++++++++++++++++++++-----------------------
> 1 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 4f94073..51dd9ac 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -1217,7 +1217,8 @@ static void rtl8169_update_counters(struct net_device *dev)
> if ((RTL_R8(ChipCmd) & CmdRxEnb) == 0)
> return;
>
> - counters = pci_alloc_consistent(tp->pci_dev, sizeof(*counters), &paddr);
> + counters = dma_alloc_coherent(&tp->pci_dev->dev, sizeof(*counters),
> + &paddr, GFP_KERNEL);
> if (!counters)
> return;
>
> @@ -1238,7 +1239,8 @@ static void rtl8169_update_counters(struct net_device *dev)
> RTL_W32(CounterAddrLow, 0);
> RTL_W32(CounterAddrHigh, 0);
>
> - pci_free_consistent(tp->pci_dev, sizeof(*counters), counters, paddr);
> + dma_free_coherent(&tp->pci_dev->dev, sizeof(*counters), counters,
> + paddr);
> }
>
> static void rtl8169_get_ethtool_stats(struct net_device *dev,
> @@ -3298,15 +3300,15 @@ static int rtl8169_open(struct net_device *dev)
>
> /*
> * Rx and Tx desscriptors needs 256 bytes alignment.
> - * pci_alloc_consistent provides more.
> + * dma_alloc_coherent provides more.
> */
> - tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
> - &tp->TxPhyAddr);
> + tp->TxDescArray = dma_alloc_coherent(&pdev->dev, R8169_TX_RING_BYTES,
> + &tp->TxPhyAddr, GFP_KERNEL);
> if (!tp->TxDescArray)
> goto err_pm_runtime_put;
>
> - tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
> - &tp->RxPhyAddr);
> + tp->RxDescArray = dma_alloc_coherent(&pdev->dev, R8169_RX_RING_BYTES,
> + &tp->RxPhyAddr, GFP_KERNEL);
> if (!tp->RxDescArray)
> goto err_free_tx_0;
>
> @@ -3340,12 +3342,12 @@ out:
> err_release_ring_2:
> rtl8169_rx_clear(tp);
> err_free_rx_1:
> - pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
> - tp->RxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
> + tp->RxPhyAddr);
> tp->RxDescArray = NULL;
> err_free_tx_0:
> - pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
> - tp->TxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
> + tp->TxPhyAddr);
> tp->TxDescArray = NULL;
> err_pm_runtime_put:
> pm_runtime_put_noidle(&pdev->dev);
> @@ -3981,7 +3983,7 @@ static void rtl8169_free_rx_skb(struct rtl8169_private *tp,
> {
> struct pci_dev *pdev = tp->pci_dev;
>
> - pci_unmap_single(pdev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
> + dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
> PCI_DMA_FROMDEVICE);
> dev_kfree_skb(*sk_buff);
> *sk_buff = NULL;
> @@ -4020,7 +4022,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
>
> skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
>
> - mapping = pci_map_single(pdev, skb->data, rx_buf_sz,
> + mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
> PCI_DMA_FROMDEVICE);
>
> rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
> @@ -4105,7 +4107,8 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
> {
> unsigned int len = tx_skb->len;
>
> - pci_unmap_single(pdev, le64_to_cpu(desc->addr), len, PCI_DMA_TODEVICE);
> + dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), len,
> + PCI_DMA_TODEVICE);
> desc->opts1 = 0x00;
> desc->opts2 = 0x00;
> desc->addr = 0x00;
> @@ -4249,7 +4252,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
> txd = tp->TxDescArray + entry;
> len = frag->size;
> addr = ((void *) page_address(frag->page)) + frag->page_offset;
> - mapping = pci_map_single(tp->pci_dev, addr, len, PCI_DMA_TODEVICE);
> + mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
> + PCI_DMA_TODEVICE);
>
> /* anti gcc 2.95.3 bugware (sic) */
> status = opts1 | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
> @@ -4319,7 +4323,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
> tp->tx_skb[entry].skb = skb;
> }
>
> - mapping = pci_map_single(tp->pci_dev, skb->data, len, PCI_DMA_TODEVICE);
> + mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
> + PCI_DMA_TODEVICE);
>
> tp->tx_skb[entry].len = len;
> txd->addr = cpu_to_le64(mapping);
> @@ -4482,8 +4487,8 @@ static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
> if (!skb)
> goto out;
>
> - pci_dma_sync_single_for_cpu(tp->pci_dev, addr, pkt_size,
> - PCI_DMA_FROMDEVICE);
> + dma_sync_single_for_cpu(&tp->pci_dev->dev, addr, pkt_size,
> + PCI_DMA_FROMDEVICE);
> skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
> *sk_buff = skb;
> done = true;
> @@ -4552,11 +4557,11 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
> }
>
> if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
> - pci_dma_sync_single_for_device(pdev, addr,
> + dma_sync_single_for_device(&pdev->dev, addr,
> pkt_size, PCI_DMA_FROMDEVICE);
> rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
> } else {
> - pci_unmap_single(pdev, addr, tp->rx_buf_sz,
> + dma_unmap_single(&pdev->dev, addr, tp->rx_buf_sz,
> PCI_DMA_FROMDEVICE);
> tp->Rx_skbuff[entry] = NULL;
> }
> @@ -4774,10 +4779,10 @@ static int rtl8169_close(struct net_device *dev)
>
> free_irq(dev->irq, dev);
>
> - pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
> - tp->RxPhyAddr);
> - pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
> - tp->TxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
> + tp->RxPhyAddr);
> + dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
> + tp->TxPhyAddr);
> tp->TxDescArray = NULL;
> tp->RxDescArray = NULL;
>
> --
> 1.7.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards,
Denis
^ permalink raw reply
* Re: [PATCH 7/8] net: Allow setting the network namespace by fd
From: David Lamparter @ 2010-09-23 14:58 UTC (permalink / raw)
To: jamal
Cc: Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Pavel Emelyanov, Ben Greear,
Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
Jan Engelhardt, Patrick McHardy
In-Reply-To: <1285240926.5036.7.camel@bigi>
On Thu, Sep 23, 2010 at 07:22:06AM -0400, jamal wrote:
> On Thu, 2010-09-23 at 01:51 -0700, Eric W. Biederman wrote:
> > Take advantage of the new abstraction and allow network devices
> > to be placed in any network namespace that we have a fd to talk
> > about.
>
> So ... why just netdevice? could you allow migration of other
> net "items" eg a route table since they are all tagged by
> netns?
migrating route table entries makes no sense because
a) they refer to devices and configuration that does not exist in the
target namespace; they only make sense within their netns context
b) they are purely virtual and you get the same result from deleting and
recreating them.
Network devices are special because they may have something attached to
them, be it hardware or some daemon.
-David
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: David Lamparter @ 2010-09-23 14:54 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: hadi, Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Ben Greear, Matt Helsley, Jonathan Corbet,
Sukadev Bhattiprolu, Jan Engelhardt, Patrick McHardy
In-Reply-To: <4C9B495D.70200@parallels.com>
On Thu, Sep 23, 2010 at 04:34:37PM +0400, Pavel Emelyanov wrote:
> On 09/23/2010 04:11 PM, jamal wrote:
> > On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
> >
> >> Why does it matter? You told, that the usage scenario was to
> >> add routes to container. If I do 2 syscalls instead of 1, is
> >> it THAT worse?
> >>
> >
> > Anything to do with socket IO that requires namespace awareness
> > applies for usage; it could be tcp/udp/etc socket. If it doesnt
> > make any difference performance wise using one scheme vs other
> > to write/read heavy messages then i dont see an issue and socketat
> > is redundant.
>
> That's what my point is about - unless we know why would we need it
> we don't need it.
>
> Eric, please clarify, what is the need in creating a socket in foreign
> net namespace?
Hmm. If you somewhere get the fd to a socket from another namespace, it
definitely does work (I'm currently implementing my "socketat" with fd
passing through AF_UNIX sockets, so i know it works), so the
setns(other...)
fd = socket(...)
setns(orig...)
sequence would certainly work. However, there might be other things
happening inbetween like a signal (imagine AIO particularly). While
signals are user-controllable (and therefore to be managed/excluded by
the user), we need to think if there are other problems with doing this
as sequence?
If there are no other problematic conditions with this, socketat should
probably be moved to a user library.
-David
^ permalink raw reply
* Re: [RFC PATCH] dont create cached routes from ARP requests
From: Ulrich Weber @ 2010-09-23 14:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100922.203442.233700254.davem@davemloft.net>
On Wed, Sep 22, 2010 at 08:34:42PM -0700, David Miller wrote:
> From: Ulrich Weber <uweber@astaro.com>
> Date: Wed, 22 Sep 2010 18:22:09 +0200
>
> > Background: At home I have two Internet connections, DSL and Cable.
> > DSL is the primary uplink while Cable is the secondary.
> > My Cable ISP is flooding me with ARP request from 10.0.0.0/8,
> > which creates routes via the primary uplink.
> > There are thousands of cached routes and after some time
> > I get "Neighbour table overflow" messages.
>
> If you get neighbour table overflows, something is holding a reference
> to the routing cache entry and/or the neighbour entries those routing
> cache entries are attached to.
>
> If these really are transient entries, they should be trivially
> garbage collected and not cause any problems at all.
rt_garbage_collect is not called within rt_intern_hash,
because the call is done within softirq context.
Forcing the call of rt_garbage_collect didn't help either,
there are no routes freed afterwards...
Cheers
Ulrich
^ permalink raw reply
* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Eric Dumazet @ 2010-09-23 14:41 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Oliver Hartkopp, Michael S. Tsirkin, netdev
In-Reply-To: <AANLkTik4d76GBa9Jwn9BOnC07_0CnMsy-XoKM_MhF_sc@mail.gmail.com>
Le jeudi 23 septembre 2010 à 22:17 +0800, Changli Gao a écrit :
> On Thu, Sep 23, 2010 at 8:29 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le jeudi 23 septembre 2010 à 18:15 +0800, Changli Gao a écrit :
> >> Since skb->destructor() is used to account socket memory, and maybe called
> >> before the skb is sent out, a corrupt skb maybe sent out finally.
> >>
> >> A new destructor is added into structure skb_shared_info(), and it won't
> >> be called until the last reference to the data of an skb is put. af_packet
> >> uses this destructor instead.
> >>
> >> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> >> ---
> >> v3: rename destructor to data_destructor, destructor_arg to data_destructor_arg,
> >> fix splice the skbs generated by AF_PACKET socket to the pipe.
> >
> > I dont understand this.
> >
> > Could you describe how splice(from af_packet to pipe) is possible with
> > af_packet send path ?
>
> af_packet sends packets to lo(127.0.0.1), and a local socket is
> receiving packets via splice.
Ouch
I am pretty sure too many things will break if we allow such packets to
get back in input path.
(think of tcp coalescing for example...)
^ permalink raw reply
* Re: [PATCH 7/8] net: Allow setting the network namespace by fd
From: Brian Haley @ 2010-09-23 14:22 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-kernel, Linux Containers, netdev, netfilter-devel,
linux-fsdevel, jamal, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Pavel Emelyanov, Ben Greear,
Matt Helsley, Jonathan Corbet, Sukadev Bhattiprolu,
Jan Engelhardt, Patrick McHardy
In-Reply-To: <m1hbhgq1v1.fsf@fess.ebiederm.org>
On 09/23/2010 04:51 AM, Eric W. Biederman wrote:
>
> Take advantage of the new abstraction and allow network devices
> to be placed in any network namespace that we have a fd to talk
> about.
>
...
> +struct net *get_net_ns_by_fd(int fd)
> +{
> + struct proc_inode *ei;
> + struct file *file;
> + struct net *net;
> +
> + file = NULL;
No need to initialize this.
> + net = ERR_PTR(-EINVAL);
or this?
> + file = proc_ns_fget(fd);
> + if (!fd)
> + goto out;
> + return ERR_PTR(-EINVAL);
Shouldn't this be:
if (!file)
And the "goto" seems wrong, especially without a {} here. Unless you
meant to keep the "goto" and branch below?
-Brian
> +
> + ei = PROC_I(file->f_dentry->d_inode);
> + if (ei->ns_ops != &netns_operations)
> + goto out;
> +
> + net = get_net(ei->ns);
> +out:
> + if (file)
> + fput(file);
> + return net;
> +}
^ permalink raw reply
* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Changli Gao @ 2010-09-23 14:17 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Oliver Hartkopp, Michael S. Tsirkin, netdev
In-Reply-To: <1285244970.2864.46.camel@edumazet-laptop>
On Thu, Sep 23, 2010 at 8:29 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 23 septembre 2010 à 18:15 +0800, Changli Gao a écrit :
>> Since skb->destructor() is used to account socket memory, and maybe called
>> before the skb is sent out, a corrupt skb maybe sent out finally.
>>
>> A new destructor is added into structure skb_shared_info(), and it won't
>> be called until the last reference to the data of an skb is put. af_packet
>> uses this destructor instead.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> v3: rename destructor to data_destructor, destructor_arg to data_destructor_arg,
>> fix splice the skbs generated by AF_PACKET socket to the pipe.
>
> I dont understand this.
>
> Could you describe how splice(from af_packet to pipe) is possible with
> af_packet send path ?
af_packet sends packets to lo(127.0.0.1), and a local socket is
receiving packets via splice.
>
> Also, on such risky patch, could you please avoid inserting cleanups ?
> I am referring to these bits :
>
OK. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH v4 2/2] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-09-23 14:16 UTC (permalink / raw)
To: Ville Tervo
Cc: Jiri Kosina, Stefan Achatz, Antonio Ospite, Alexey Dobriyan,
Tejun Heo, Alan Stern, Greg Kroah-Hartman, Marcel Holtmann,
Stephane Chatty, Michael Poole, David S. Miller, Bastien Nocera,
Eric Dumazet, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20100923115108.GC2379@null>
On Sep 23, 2010, at 7:51 AM, Ville Tervo wrote:
> Hi Alan,
>
> One comment.
>
> How about a variable called ret and using that to return len or
> errno? It
> would eliminate code dublication.
>
Hi Ville,
Where specifically? In which function? I've gone through it a couple
of times and failed to find return statements which are superfluous.
Maybe I'm missing something fundamental?
Alan.
^ permalink raw reply
* RE: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
From: Xin, Xiaohui @ 2010-09-23 12:56 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mingo@elte.hu, davem@davemloft.net,
herbert@gondor.hengli.com.au, jdike@linux.intel.com
In-Reply-To: <20100922115505.GF16423@redhat.com>
>-----Original Message-----
>From: Michael S. Tsirkin [mailto:mst@redhat.com]
>Sent: Wednesday, September 22, 2010 7:55 PM
>To: Xin, Xiaohui
>Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>jdike@linux.intel.com
>Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
>
>On Wed, Sep 22, 2010 at 07:41:36PM +0800, Xin, Xiaohui wrote:
>> >-----Original Message-----
>> >From: Michael S. Tsirkin [mailto:mst@redhat.com]
>> >Sent: Tuesday, September 21, 2010 9:14 PM
>> >To: Xin, Xiaohui
>> >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>> >mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>> >jdike@linux.intel.com
>> >Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
>> >
>> >On Tue, Sep 21, 2010 at 09:39:31AM +0800, Xin, Xiaohui wrote:
>> >> >From: Michael S. Tsirkin [mailto:mst@redhat.com]
>> >> >Sent: Monday, September 20, 2010 7:37 PM
>> >> >To: Xin, Xiaohui
>> >> >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>> >> >mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
>> >> >jdike@linux.intel.com
>> >> >Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
>> >> >
>> >> >On Mon, Sep 20, 2010 at 04:08:48PM +0800, xiaohui.xin@intel.com wrote:
>> >> >> From: Xin Xiaohui <xiaohui.xin@intel.com>
>> >> >>
>> >> >> ---
>> >> >> Michael,
>> >> >> I have move the ioctl to configure the locked memory to vhost
>> >> >
>> >> >It's ok to move this to vhost but vhost does not
>> >> >know how much memory is needed by the backend.
>> >>
>> >> I think the backend here you mean is mp device.
>> >> Actually, the memory needed is related to vq->num to run zero-copy
>> >> smoothly.
>> >> That means mp device did not know it but vhost did.
>> >
>> >Well, this might be so if you insist on locking
>> >all posted buffers immediately. However, let's assume I have a
>> >very large ring and prepost a ton of RX buffers:
>> >there's no need to lock all of them directly:
>> >
>> >if we have buffers A and B, we can lock A, pass it
>> >to hardware, and when A is consumed unlock A, lock B
>> >and pass it to hardware.
>> >
>> >
>> >It's not really critical. But note we can always have userspace
>> >tell MP device all it wants to know, after all.
>> >
>> Ok. Here are two values we have mentioned, one is how much memory
>> user application wants to lock, and one is how much memory locked
>> is needed to run smoothly. When net backend is setup, we first need
>> an ioctl to get how much memory is needed to lock, and then we call
>> another ioctl to set how much it want to lock. Is that what's in your mind?
>
>That's fine.
>
>> >> And the rlimt stuff is per process, we use current pointer to set
>> >> and check the rlimit, the operations should be in the same process.
>> >
>> >Well no, the ring is handled from the kernel thread: we switch the mm to
>> >point to the owner task so copy from/to user and friends work, but you
>> >can't access the rlimit etc.
>> >
>> Yes, the userspace and vhost kernel is not the same process. But we can
>> record the task pointer as mm.
>
>So you will have to store mm and do device->mm, not current->mm.
>Anyway, better not touch mm on data path.
>
>> >> Now the check operations are in vhost process, as mp_recvmsg() or
>> >> mp_sendmsg() are called by vhost.
>> >
>> >Hmm, what do you mean by the check operations?
>> >send/recv are data path operations, they shouldn't
>> >do any checks, should they?
>> >
>> As you mentioned what infiniband driver done:
>> down_write(¤t->mm->mmap_sem);
>>
>> locked = npages + current->mm->locked_vm;
>> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>>
>> if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) {
>> ret = -ENOMEM;
>> goto out;
>> }
>>
>> cur_base = addr & PAGE_MASK;
>>
>> ret = 0;
>> while (npages) {
>> ret = get_user_pages(current, current->mm, cur_base,
>> min_t(unsigned long, npages,
>> PAGE_SIZE / sizeof (struct page *)),
>> 1, !umem->writable, page_list, vma_list);
>>
>> I think it's a data path too.
>
>in infiniband this is used to 'register memory' which is not data path.
>
>> We do the check because get_user_pages() really pin and locked
>> the memory.
>
>Don't do this. Performance will be bad.
>Do the check once in ioctl and increment locked_vm by max amount you will use.
>On data path just make sure you do not exceed what userspace told you
>to.
What's in my mind is that in the ioctl which to get the memory locked needed to run smoothly,
it just return a value of how much memory is needed by mp device.
And then in the ioctl which to set the memory locked by user space, it check the rlimit and
increment locked_vm by user want. But I'm not sure how to "make sure do not exceed what
userspace told to". If we don't check locked_vm, what do we use to check? And Is it another kind of check on data path?
>
>>
>> >> So set operations should be in
>> >> vhost process too, it's natural.
>> >>
>> >> >So I think we'll need another ioctl in the backend
>> >> >to tell userspace how much memory is needed?
>> >> >
>> >> Except vhost tells it to mp device, mp did not know
>> >> how much memory is needed to run zero-copy smoothly.
>> >> Is userspace interested about the memory mp is needed?
>> >
>> >Couldn't parse this last question.
>> >I think userspace generally does want control over
>> >how much memory we'll lock. We should not just lock
>> >as much as we can.
>> >
>> >--
>> >MST
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: Pavel Emelyanov @ 2010-09-23 12:34 UTC (permalink / raw)
To: hadi-fAAogVwAN2Kw5LPnMra/2Q, Eric W. Biederman
Cc: Sukadev Bhattiprolu, Jonathan Corbet, Ulrich Drepper,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jan Engelhardt,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk,
Linux Containers, Ben Greear, Linus Torvalds, David Miller,
Al Viro
In-Reply-To: <1285243881.5036.22.camel@bigi>
On 09/23/2010 04:11 PM, jamal wrote:
> On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
>
>> Why does it matter? You told, that the usage scenario was to
>> add routes to container. If I do 2 syscalls instead of 1, is
>> it THAT worse?
>>
>
> Anything to do with socket IO that requires namespace awareness
> applies for usage; it could be tcp/udp/etc socket. If it doesnt
> make any difference performance wise using one scheme vs other
> to write/read heavy messages then i dont see an issue and socketat
> is redundant.
That's what my point is about - unless we know why would we need it
we don't need it.
Eric, please clarify, what is the need in creating a socket in foreign
net namespace?
^ permalink raw reply
* Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out
From: Eric Dumazet @ 2010-09-23 12:29 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Oliver Hartkopp, Michael S. Tsirkin, netdev
In-Reply-To: <1285236939-3239-1-git-send-email-xiaosuo@gmail.com>
Le jeudi 23 septembre 2010 à 18:15 +0800, Changli Gao a écrit :
> Since skb->destructor() is used to account socket memory, and maybe called
> before the skb is sent out, a corrupt skb maybe sent out finally.
>
> A new destructor is added into structure skb_shared_info(), and it won't
> be called until the last reference to the data of an skb is put. af_packet
> uses this destructor instead.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> v3: rename destructor to data_destructor, destructor_arg to data_destructor_arg,
> fix splice the skbs generated by AF_PACKET socket to the pipe.
I dont understand this.
Could you describe how splice(from af_packet to pipe) is possible with
af_packet send path ?
Also, on such risky patch, could you please avoid inserting cleanups ?
I am referring to these bits :
@@ -884,9 +883,8 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
to_write = tp_len;
if (sock->type == SOCK_DGRAM) {
- err = dev_hard_header(skb, dev, ntohs(proto), addr,
- NULL, tp_len);
- if (unlikely(err < 0))
+ if (unlikely(dev_hard_header(skb, dev, ntohs(proto), addr,
+ NULL, tp_len) < 0))
return -EINVAL;
} else if (dev->hard_header_len) {
/* net device doesn't like empty head */
@@ -897,8 +895,7 @@ static int tpacket_fill_skb(struct packet_sock *po,
struct sk_buff *skb,
}
skb_push(skb, dev->hard_header_len);
- err = skb_store_bits(skb, 0, data,
- dev->hard_header_len);
+ err = skb_store_bits(skb, 0, data, dev->hard_header_len);
if (unlikely(err))
return err;
@@ -906,7 +903,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
to_write -= dev->hard_header_len;
}
- err = -EFAULT;
page = virt_to_page(data);
offset = offset_in_page(data);
len_max = PAGE_SIZE - offset;
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: jamal @ 2010-09-23 12:11 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Ben Greear, Matt Helsley,
Jonathan Corbet, Sukadev Bhattiprolu, Jan Engelhardt,
Patrick McHardy
In-Reply-To: <4C9B3F9C.8080506@parallels.com>
On Thu, 2010-09-23 at 15:53 +0400, Pavel Emelyanov wrote:
> Why does it matter? You told, that the usage scenario was to
> add routes to container. If I do 2 syscalls instead of 1, is
> it THAT worse?
>
Anything to do with socket IO that requires namespace awareness
applies for usage; it could be tcp/udp/etc socket. If it doesnt
make any difference performance wise using one scheme vs other
to write/read heavy messages then i dont see an issue and socketat
is redundant.
If i was to pick blindly - I would say whatever approach with
less syscalls is better even if just a "slow" path one time
thing. I could create a scenario which would make it bad
to have more syscalls.
But theres also the simplicity aspect in doing:
fdx = socketat namespace foo
use fdx for read/write/poll into foo without any wrapper code.
Vs
enter foo
fdx = socket ..
read/write fdx
leave foo.
> Just like it used to before the enter.
>
So if i enter foo, get a fdx, leave foo i can use it in
ns0 as if it was in ns0?
cheers,
jamal
^ permalink raw reply
* [PATCH 2/2 -next] r8169: use device model DMA API
From: Stanislaw Gruszka @ 2010-09-23 12:01 UTC (permalink / raw)
To: Francois Romieu, netdev; +Cc: Stanislaw Gruszka
In-Reply-To: <1285243291-4520-1-git-send-email-sgruszka@redhat.com>
Use DMA API as PCI equivalents will be deprecated. This change also
allow to allocate with GFP_KERNEL where possible.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/r8169.c | 53 +++++++++++++++++++++++++++-----------------------
1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 4f94073..51dd9ac 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1217,7 +1217,8 @@ static void rtl8169_update_counters(struct net_device *dev)
if ((RTL_R8(ChipCmd) & CmdRxEnb) == 0)
return;
- counters = pci_alloc_consistent(tp->pci_dev, sizeof(*counters), &paddr);
+ counters = dma_alloc_coherent(&tp->pci_dev->dev, sizeof(*counters),
+ &paddr, GFP_KERNEL);
if (!counters)
return;
@@ -1238,7 +1239,8 @@ static void rtl8169_update_counters(struct net_device *dev)
RTL_W32(CounterAddrLow, 0);
RTL_W32(CounterAddrHigh, 0);
- pci_free_consistent(tp->pci_dev, sizeof(*counters), counters, paddr);
+ dma_free_coherent(&tp->pci_dev->dev, sizeof(*counters), counters,
+ paddr);
}
static void rtl8169_get_ethtool_stats(struct net_device *dev,
@@ -3298,15 +3300,15 @@ static int rtl8169_open(struct net_device *dev)
/*
* Rx and Tx desscriptors needs 256 bytes alignment.
- * pci_alloc_consistent provides more.
+ * dma_alloc_coherent provides more.
*/
- tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
- &tp->TxPhyAddr);
+ tp->TxDescArray = dma_alloc_coherent(&pdev->dev, R8169_TX_RING_BYTES,
+ &tp->TxPhyAddr, GFP_KERNEL);
if (!tp->TxDescArray)
goto err_pm_runtime_put;
- tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
- &tp->RxPhyAddr);
+ tp->RxDescArray = dma_alloc_coherent(&pdev->dev, R8169_RX_RING_BYTES,
+ &tp->RxPhyAddr, GFP_KERNEL);
if (!tp->RxDescArray)
goto err_free_tx_0;
@@ -3340,12 +3342,12 @@ out:
err_release_ring_2:
rtl8169_rx_clear(tp);
err_free_rx_1:
- pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
- tp->RxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
+ tp->RxPhyAddr);
tp->RxDescArray = NULL;
err_free_tx_0:
- pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
- tp->TxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
+ tp->TxPhyAddr);
tp->TxDescArray = NULL;
err_pm_runtime_put:
pm_runtime_put_noidle(&pdev->dev);
@@ -3981,7 +3983,7 @@ static void rtl8169_free_rx_skb(struct rtl8169_private *tp,
{
struct pci_dev *pdev = tp->pci_dev;
- pci_unmap_single(pdev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
+ dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), tp->rx_buf_sz,
PCI_DMA_FROMDEVICE);
dev_kfree_skb(*sk_buff);
*sk_buff = NULL;
@@ -4020,7 +4022,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
skb_reserve(skb, align ? ((pad - 1) & (unsigned long)skb->data) : pad);
- mapping = pci_map_single(pdev, skb->data, rx_buf_sz,
+ mapping = dma_map_single(&pdev->dev, skb->data, rx_buf_sz,
PCI_DMA_FROMDEVICE);
rtl8169_map_to_asic(desc, mapping, rx_buf_sz);
@@ -4105,7 +4107,8 @@ static void rtl8169_unmap_tx_skb(struct pci_dev *pdev, struct ring_info *tx_skb,
{
unsigned int len = tx_skb->len;
- pci_unmap_single(pdev, le64_to_cpu(desc->addr), len, PCI_DMA_TODEVICE);
+ dma_unmap_single(&pdev->dev, le64_to_cpu(desc->addr), len,
+ PCI_DMA_TODEVICE);
desc->opts1 = 0x00;
desc->opts2 = 0x00;
desc->addr = 0x00;
@@ -4249,7 +4252,8 @@ static int rtl8169_xmit_frags(struct rtl8169_private *tp, struct sk_buff *skb,
txd = tp->TxDescArray + entry;
len = frag->size;
addr = ((void *) page_address(frag->page)) + frag->page_offset;
- mapping = pci_map_single(tp->pci_dev, addr, len, PCI_DMA_TODEVICE);
+ mapping = dma_map_single(&tp->pci_dev->dev, addr, len,
+ PCI_DMA_TODEVICE);
/* anti gcc 2.95.3 bugware (sic) */
status = opts1 | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
@@ -4319,7 +4323,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
tp->tx_skb[entry].skb = skb;
}
- mapping = pci_map_single(tp->pci_dev, skb->data, len, PCI_DMA_TODEVICE);
+ mapping = dma_map_single(&tp->pci_dev->dev, skb->data, len,
+ PCI_DMA_TODEVICE);
tp->tx_skb[entry].len = len;
txd->addr = cpu_to_le64(mapping);
@@ -4482,8 +4487,8 @@ static inline bool rtl8169_try_rx_copy(struct sk_buff **sk_buff,
if (!skb)
goto out;
- pci_dma_sync_single_for_cpu(tp->pci_dev, addr, pkt_size,
- PCI_DMA_FROMDEVICE);
+ dma_sync_single_for_cpu(&tp->pci_dev->dev, addr, pkt_size,
+ PCI_DMA_FROMDEVICE);
skb_copy_from_linear_data(*sk_buff, skb->data, pkt_size);
*sk_buff = skb;
done = true;
@@ -4552,11 +4557,11 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
}
if (rtl8169_try_rx_copy(&skb, tp, pkt_size, addr)) {
- pci_dma_sync_single_for_device(pdev, addr,
+ dma_sync_single_for_device(&pdev->dev, addr,
pkt_size, PCI_DMA_FROMDEVICE);
rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
} else {
- pci_unmap_single(pdev, addr, tp->rx_buf_sz,
+ dma_unmap_single(&pdev->dev, addr, tp->rx_buf_sz,
PCI_DMA_FROMDEVICE);
tp->Rx_skbuff[entry] = NULL;
}
@@ -4774,10 +4779,10 @@ static int rtl8169_close(struct net_device *dev)
free_irq(dev->irq, dev);
- pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
- tp->RxPhyAddr);
- pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
- tp->TxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
+ tp->RxPhyAddr);
+ dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
+ tp->TxPhyAddr);
tp->TxDescArray = NULL;
tp->RxDescArray = NULL;
--
1.7.0.1
^ permalink raw reply related
* [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Stanislaw Gruszka @ 2010-09-23 12:01 UTC (permalink / raw)
To: Francois Romieu, netdev; +Cc: Stanislaw Gruszka
We have fedora bug report where driver fail to initialize after
suspend/resume because of memory allocation errors:
https://bugzilla.redhat.com/show_bug.cgi?id=629158
To fix use GFP_KERNEL allocation where possible.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/r8169.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 5490033..4f94073 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4006,7 +4006,7 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
struct net_device *dev,
struct RxDesc *desc, int rx_buf_sz,
- unsigned int align)
+ unsigned int align, gfp_t gfp)
{
struct sk_buff *skb;
dma_addr_t mapping;
@@ -4014,7 +4014,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
pad = align ? align : NET_IP_ALIGN;
- skb = netdev_alloc_skb(dev, rx_buf_sz + pad);
+ skb = __netdev_alloc_skb(dev, rx_buf_sz + pad, gfp);
if (!skb)
goto err_out;
@@ -4045,7 +4045,7 @@ static void rtl8169_rx_clear(struct rtl8169_private *tp)
}
static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
- u32 start, u32 end)
+ u32 start, u32 end, gfp_t gfp)
{
u32 cur;
@@ -4060,7 +4060,7 @@ static u32 rtl8169_rx_fill(struct rtl8169_private *tp, struct net_device *dev,
skb = rtl8169_alloc_rx_skb(tp->pci_dev, dev,
tp->RxDescArray + i,
- tp->rx_buf_sz, tp->align);
+ tp->rx_buf_sz, tp->align, gfp);
if (!skb)
break;
@@ -4088,7 +4088,7 @@ static int rtl8169_init_ring(struct net_device *dev)
memset(tp->tx_skb, 0x0, NUM_TX_DESC * sizeof(struct ring_info));
memset(tp->Rx_skbuff, 0x0, NUM_RX_DESC * sizeof(struct sk_buff *));
- if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC) != NUM_RX_DESC)
+ if (rtl8169_rx_fill(tp, dev, 0, NUM_RX_DESC, GFP_KERNEL) != NUM_RX_DESC)
goto err_out;
rtl8169_mark_as_last_descriptor(tp->RxDescArray + NUM_RX_DESC - 1);
@@ -4587,7 +4587,7 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
count = cur_rx - tp->cur_rx;
tp->cur_rx = cur_rx;
- delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx);
+ delta = rtl8169_rx_fill(tp, dev, tp->dirty_rx, tp->cur_rx, GFP_ATOMIC);
if (!delta && count)
netif_info(tp, intr, dev, "no Rx buffer allocated\n");
tp->dirty_rx += delta;
--
1.7.0.1
^ permalink raw reply related
* Re: [PATCH 8/8] net: Implement socketat.
From: Pavel Emelyanov @ 2010-09-23 11:53 UTC (permalink / raw)
To: hadi
Cc: Eric W. Biederman, linux-kernel, Linux Containers, netdev,
netfilter-devel, linux-fsdevel, Daniel Lezcano, Linus Torvalds,
Michael Kerrisk, Ulrich Drepper, Al Viro, David Miller,
Serge E. Hallyn, Pavel Emelyanov, Ben Greear, Matt Helsley,
Jonathan Corbet, Sukadev Bhattiprolu, Jan Engelhardt,
Patrick McHardy
In-Reply-To: <1285242055.5036.9.camel@bigi>
On 09/23/2010 03:40 PM, jamal wrote:
> On Thu, 2010-09-23 at 15:33 +0400, Pavel Emelyanov wrote:
>
>> This particular usecase is unneeded once you have the "enter" ability.
>
> Is that cheaper from a syscall count/cost?
Why does it matter? You told, that the usage scenario was to
add routes to container. If I do 2 syscalls instead of 1, is
it THAT worse?
> i.e do I have to enter every time i want to write/read this fd?
No - you enter once, create a socket and do whatever you need
withing the enterned namespace.
> How does poll/select work in that enter scenario?
Just like it used to before the enter.
> cheers,
> jamal
>
>
^ permalink raw reply
* [PATCH] br2684: fix scheduling while atomic
From: Karl Hiramoto @ 2010-09-23 11:50 UTC (permalink / raw)
To: netdev, mmvinni; +Cc: davem, chas, Karl Hiramoto
In-Reply-To: <20100922144329.0cb3cf86@thirdoffive.cmf.nrl.navy.mil>
You can't call atomic_notifier_chain_unregister() while in atomic context.
Fix, call un/register_atmdevice_notifier in module __init and __exit.
Bug report:
http://comments.gmane.org/gmane.linux.network/172603
Signed-off-by: Karl Hiramoto <karl@hiramoto.org>
---
net/atm/br2684.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index 651babd..ad2b232 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -399,12 +399,6 @@ static void br2684_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
unregister_netdev(net_dev);
free_netdev(net_dev);
}
- read_lock_irq(&devs_lock);
- if (list_empty(&br2684_devs)) {
- /* last br2684 device */
- unregister_atmdevice_notifier(&atm_dev_notifier);
- }
- read_unlock_irq(&devs_lock);
return;
}
@@ -675,7 +669,6 @@ static int br2684_create(void __user *arg)
if (list_empty(&br2684_devs)) {
/* 1st br2684 device */
- register_atmdevice_notifier(&atm_dev_notifier);
brdev->number = 1;
} else
brdev->number = BRPRIV(list_entry_brdev(br2684_devs.prev))->number + 1;
@@ -815,6 +808,7 @@ static int __init br2684_init(void)
return -ENOMEM;
#endif
register_atm_ioctl(&br2684_ioctl_ops);
+ register_atmdevice_notifier(&atm_dev_notifier);
return 0;
}
@@ -830,9 +824,7 @@ static void __exit br2684_exit(void)
#endif
- /* if not already empty */
- if (!list_empty(&br2684_devs))
- unregister_atmdevice_notifier(&atm_dev_notifier);
+ unregister_atmdevice_notifier(&atm_dev_notifier);
while (!list_empty(&br2684_devs)) {
net_dev = list_entry_brdev(br2684_devs.next);
--
1.7.2.2
^ permalink raw reply related
* Re: [PATCH v4 2/2] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Ville Tervo @ 2010-09-23 11:51 UTC (permalink / raw)
To: ext Alan Ott
Cc: Jiri Kosina, Stefan Achatz, Antonio Ospite, Alexey Dobriyan,
Tejun Heo, Alan Stern, Greg Kroah-Hartman, Marcel Holtmann,
Stephane Chatty, Michael Poole, David S. Miller, Bastien Nocera,
Eric Dumazet, linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1281990059-3562-3-git-send-email-alan-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org>
Hi Alan,
One comment.
On Mon, Aug 16, 2010 at 10:20:59PM +0200, ext Alan Ott wrote:
> This patch adds support or getting and setting feature reports for bluetooth
> HID devices from HIDRAW.
>
> Signed-off-by: Alan Ott <alan-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org>
> ---
> net/bluetooth/hidp/core.c | 114 +++++++++++++++++++++++++++++++++++++++++++--
> net/bluetooth/hidp/hidp.h | 8 +++
> 2 files changed, 118 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index bfe641b..0e4880e 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -36,6 +36,7 @@
> #include <linux/file.h>
> #include <linux/init.h>
> #include <linux/wait.h>
> +#include <linux/mutex.h>
> #include <net/sock.h>
>
> #include <linux/input.h>
> @@ -313,6 +314,86 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep
> return hidp_queue_report(session, buf, rsize);
> }
>
> +static int hidp_get_raw_report(struct hid_device *hid,
> + unsigned char report_number,
> + unsigned char *data, size_t count,
> + unsigned char report_type)
> +{
> + struct hidp_session *session = hid->driver_data;
> + struct sk_buff *skb;
> + size_t len;
> + int numbered_reports = hid->report_enum[report_type].numbered;
> +
> + switch (report_type) {
> + case HID_FEATURE_REPORT:
> + report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_FEATURE;
> + break;
> + case HID_INPUT_REPORT:
> + report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_INPUT;
> + break;
> + case HID_OUTPUT_REPORT:
> + report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_OUPUT;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (mutex_lock_interruptible(&session->report_mutex))
> + return -ERESTARTSYS;
> +
> + /* Set up our wait, and send the report request to the device. */
> + session->waiting_report_type = report_type & HIDP_DATA_RTYPE_MASK;
> + session->waiting_report_number = numbered_reports ? report_number : -1;
> + set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + data[0] = report_number;
> + if (hidp_send_ctrl_message(hid->driver_data, report_type, data, 1))
> + goto err_eio;
> +
> + /* Wait for the return of the report. The returned report
> + gets put in session->report_return. */
> + while (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags)) {
> + int res;
> +
> + res = wait_event_interruptible_timeout(session->report_queue,
> + !test_bit(HIDP_WAITING_FOR_RETURN, &session->flags),
> + 5*HZ);
> + if (res == 0) {
> + /* timeout */
> + goto err_eio;
> + }
> + if (res < 0) {
> + /* signal */
> + goto err_restartsys;
> + }
> + }
> +
> + skb = session->report_return;
> + if (skb) {
> + len = skb->len < count ? skb->len : count;
> + memcpy(data, skb->data, len);
> +
> + kfree_skb(skb);
> + session->report_return = NULL;
> + } else {
> + /* Device returned a HANDSHAKE, indicating protocol error. */
> + len = -EIO;
> + }
> +
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + mutex_unlock(&session->report_mutex);
> +
> + return len;
> +
> +err_restartsys:
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + mutex_unlock(&session->report_mutex);
> + return -ERESTARTSYS;
> +err_eio:
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + mutex_unlock(&session->report_mutex);
> + return -EIO;
> +}
How about a variable called ret and using that to return len or errno? It
would eliminate code dublication.
--
Ville
^ permalink raw reply
* Re: [PATCH 20/20] net: change to new flag variable
From: Sjur Brændeland @ 2010-09-23 11:48 UTC (permalink / raw)
To: matt mooney
Cc: David S. Miller, John W. Linville, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <1285224715-30137-1-git-send-email-mfm@muteddisk.com>
matt mooney wrote:
> Replace EXTRA_CFLAGS with ccflags-y.
Thank you,
CAIF parts looks good to me.
Acked-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
^ permalink raw reply
* Re: idr_get_new_exact ?
From: Tejun Heo @ 2010-09-23 11:46 UTC (permalink / raw)
To: Paul Mundt
Cc: Roland Dreier, Ohad Ben-Cohen, linux-kernel,
Jean Delvare (PC drivers, core), Ben Dooks (embedded platforms),
Roland Dreier, Sean Hefty, Hal Rosenstock, Steve Wise, Neil Brown,
Paul Mackerras, linux-i2c, linux-rdma, dm-devel, linux-raid,
linux-ppp, netdev, Andrew Morton, Alasdair G Kergon
In-Reply-To: <20100923114255.GB27960@linux-sh.org>
Hello,
On 09/23/2010 01:42 PM, Paul Mundt wrote:
> On Mon, Sep 20, 2010 at 11:26:47PM +0200, Tejun Heo wrote:
>> Hello,
>>
>> On 09/20/2010 10:35 PM, Roland Dreier wrote:
>>> Looks fine to me as an improvement over the status quo, but I wonder how
>>> many of these places could use the radix_tree stuff instead? If you're
>>> not using the ability of the idr code to assign an id for you, then it
>>> seems the radix_tree API is a better fit.
>>
>> I agree. Wouldn't those users better off simply using radix tree?
>>
> It could go either way. I was about to write the same function when
> playing with it for IRQ mapping, the idea being to propagate the initial
> tree with sparse static vectors and then switch over to dynamic IDs for
> virtual IRQ creation. I ended up going with a radix tree for other
> reasons, though.
I see. If there are use cases where fixed and dynamic IDs need to be
mixed, no objection from me.
Thanks.
--
tejun
^ permalink raw reply
* Re: idr_get_new_exact ?
From: Paul Mundt @ 2010-09-23 11:42 UTC (permalink / raw)
To: Tejun Heo
Cc: Roland Dreier, Ohad Ben-Cohen,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Jean Delvare (PC drivers, core), Ben Dooks (embedded platforms),
Roland Dreier, Sean Hefty, Hal Rosenstock, Steve Wise, Neil Brown,
Paul Mackerras, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-ppp-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b
In-Reply-To: <4C97D197.9070703-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Sep 20, 2010 at 11:26:47PM +0200, Tejun Heo wrote:
> Hello,
>
> On 09/20/2010 10:35 PM, Roland Dreier wrote:
> > Looks fine to me as an improvement over the status quo, but I wonder how
> > many of these places could use the radix_tree stuff instead? If you're
> > not using the ability of the idr code to assign an id for you, then it
> > seems the radix_tree API is a better fit.
>
> I agree. Wouldn't those users better off simply using radix tree?
>
It could go either way. I was about to write the same function when
playing with it for IRQ mapping, the idea being to propagate the initial
tree with sparse static vectors and then switch over to dynamic IDs for
virtual IRQ creation. I ended up going with a radix tree for other
reasons, though.
^ permalink raw reply
* Re: [PATCH 8/8] net: Implement socketat.
From: jamal @ 2010-09-23 11:40 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Sukadev Bhattiprolu, Pavel Emelyanov, Jonathan Corbet,
Ulrich Drepper, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jan Engelhardt,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Al Viro,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
Linux Containers, Ben Greear, Linus Torvalds, David Miller,
Michael Kerrisk
In-Reply-To: <4C9B3B06.900-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Thu, 2010-09-23 at 15:33 +0400, Pavel Emelyanov wrote:
> This particular usecase is unneeded once you have the "enter" ability.
Is that cheaper from a syscall count/cost?
i.e do I have to enter every time i want to write/read this fd?
How does poll/select work in that enter scenario?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 12/13] net/sunrpc/rpc_pipe.c: Remove unnecessary casts of private_data
From: Jiri Kosina @ 2010-09-23 11:40 UTC (permalink / raw)
To: Joe Perches
Cc: J. Bruce Fields, Neil Brown, Trond Myklebust, David S. Miller,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <dda96f0e94bd038179ae11dab4d73a7ab704276b.1283650107.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
On Sat, 4 Sep 2010, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
> net/sunrpc/rpc_pipe.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
> index 95ccbcf..1f7fc50 100644
> --- a/net/sunrpc/rpc_pipe.c
> +++ b/net/sunrpc/rpc_pipe.c
> @@ -204,7 +204,7 @@ rpc_pipe_release(struct inode *inode, struct file *filp)
> mutex_lock(&inode->i_mutex);
> if (rpci->ops == NULL)
> goto out;
> - msg = (struct rpc_pipe_msg *)filp->private_data;
> + msg = filp->private_data;
> if (msg != NULL) {
> spin_lock(&inode->i_lock);
> msg->errno = -EAGAIN;
> @@ -322,7 +322,7 @@ rpc_pipe_ioctl_unlocked(struct file *filp, unsigned int cmd, unsigned long arg)
> len = rpci->pipelen;
> if (filp->private_data) {
> struct rpc_pipe_msg *msg;
> - msg = (struct rpc_pipe_msg *)filp->private_data;
> + msg = filp->private_data;
> len += msg->len - msg->copied;
> }
> return put_user(len, (int __user *)arg);
Not present in linux-next as of today. Applying to trivial queue.
--
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox