* Re: [net-next RFC v2] net_cls: traffic counter based on classification control cgroup
From: Daniel Wagner @ 2012-11-28 8:09 UTC (permalink / raw)
To: Alexey Perevalov
Cc: Glauber Costa, netdev-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <50B59F54.8080401-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Hi Alexey,
On 28.11.2012 06:21, Alexey Perevalov wrote:
>>> Daniel Wagner is working on something a lot similar.
>> Yes, basically what I try to do is explained by this excellent article
>>
>> https://lwn.net/Articles/523058/
> I read articles and agreed with aspects.
> But problem of selecting preferred network for application can be solved
> using netprio cgroup.
Choosing the which network to connect to is job of a connection manager.
I don't see how a cgroup controller can help you there. I guess I do not
understand your statement. Can you rephrase please?
>> The second implementation is adding a new iptables matcher which matches
>> on LSM contexts. Then you can do something like this:
>>
>> iptables -t mangle -A OUTPUT -m secmark --secctx
>> unconfined_u:unconfined_r:foo_t:s0-s0:c0.c1023 -j MARK --set-mark 200
> As I understand in LSM context it works for egress and ingress.
Yes, I am using CONNMARK in conjunction with the the above LSM context
matcher. I am still playing around, but it looks quite promising.
>>> 2) When Daniel exposed his use case to me, it gave me the impression
>>> that "counting traffic" is something that is totally doable by having a
>>> dedicated interface in a separate namespace. Basically, we already count
>>> traffic (rx and tx) for all interfaces anyway, so it suggests that it
>>> could be an interesting way to see the problem.
>> Moving applications into separate net namespaces is for sure a valid
>> solution.
>> Though there is a one drawback in this approach. The namespaces need
>> to be
>> attached to a bridge and then some NATting. That means every application
>> would get it's own IP address. This might be okay for your certain use
>> cases but I am still trying to work around this. Glauber and I had some
>> discussion about this and he suggested to allow the physical networking
>> device to be attached to several namespaces (e.g. via macvlan). Every
>> namespace would get the same IP address. Unfortunately, this would
>> result in
>> the same mess as several physical devices on a network get the same
>> IP address assigned.
> Is I truly understand what to make statistics works we need to put
> process to separate namespace?
If a process lives in its own network namespace then you can
count the packets/bytes on the network interface level. The side effect
is that is that each namespace is obviously a new network and has to be
treated as such.
> Approach to keep counter in cgroup hasn't such side effects, but it has
> another ).
cgroups are not for free. Currently a lot of effort is put into getting
a reasonable performance and behavior into cgroups. In this situation
any new feature added to cgroups will need a pretty good justification
why it is needed and why it cant be done with existing infrastructure.
Here is some background information on the state of cgroups:
http://thread.gmane.org/gmane.linux.kernel.containers/23698
cheers,
daniel
^ permalink raw reply
* Re: [PATCH v3 8/7] pppoatm: fix missing wakeup in pppoatm_send()
From: Krzysztof Mazur @ 2012-11-28 8:12 UTC (permalink / raw)
To: David Woodhouse; +Cc: chas williams - CONTRACTOR, netdev, linux-kernel, davem
In-Reply-To: <1354063697.21562.4.camel@shinybook.infradead.org>
On Wed, Nov 28, 2012 at 12:48:17AM +0000, David Woodhouse wrote:
> On Tue, 2012-11-27 at 10:23 -0500, chas williams - CONTRACTOR wrote:
> > yes, but dont call it 8/7 since that doesnt make sense.
>
> It made enough sense when it was a single patch appended to a thread of
> 7 other patches from Krzysztof. But now it's all got a little more
> complex, so I've tried to collect together the latest version of
> everything we've discussed:
There was also discussion about patch 9/7 "pppoatm: wakeup after ATM
unlock only when it's needed".
>
> http://git.infradead.org/users/dwmw2/atm.git
> git://git.infradead.org/users/dwmw2/atm.git
>
> David Woodhouse (5):
> atm: Add release_cb() callback to vcc
> pppoatm: fix missing wakeup in pppoatm_send()
> br2684: fix module_put() race
for the three patches above:
Acked-by: Krzysztof Mazur <krzysiek@podlesie.net>
Krzysiek
^ permalink raw reply
* Re: TCP and reordering
From: Christoph Paasch @ 2012-11-28 8:21 UTC (permalink / raw)
To: David Woodhouse; +Cc: David Miller, saku, rick.jones2, netdev
In-Reply-To: <1354089566.21562.20.camel@shinybook.infradead.org>
On Wednesday 28 November 2012 07:59:26 David Woodhouse wrote:
> My 'strange justification' for reordering, albeit not entirely on
> purpose, is that a single ADSL line at 8Mb/s down, 448Kb/s up is less
> bandwidth than I had to my dorm room 16 years ago. So I bond two of
> them, and naturally expect a certain amount of reordering.
You might want to have a look at MultiPath TCP [1], which allows the use of
multiple interfaces for a single TCP connection. It is somehow similar to
SCTP-CMT, with the difference that MPTCP is able to pass by today's firewalls
and NATs and does not require any modifications to the applications.
E.g., you could install MPTCP on your end host and set up an HTTP-proxy on a
public web hoster to terminate your MPTCP session -- as servers don't (yet)
support MPTCP, you will have to terminate the MPTCP session somewhere.
Cheers,
Christoph
[1] http://multipath-tcp.org
--
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
Université Catholique de Louvain
--
^ permalink raw reply
* Re: TCP and reordering
From: Vijay Subramanian @ 2012-11-28 8:22 UTC (permalink / raw)
To: David Woodhouse; +Cc: David Miller, saku, rick.jones2, netdev
In-Reply-To: <1354089566.21562.20.camel@shinybook.infradead.org>
>
> Short of going through whole dumps and looking, is there a good way to
> get statistics?
>
Hi David,
I don't believe reordering is tracked on the receiver side but on the
sender, there are SNMB_MIB items.
They can be tracked and can be viewed using nstat/netstat
# nstat -az | grep -i reorder
TcpExtTCPFACKReorder 0 0.0
TcpExtTCPSACKReorder 0 0.0
TcpExtTCPRenoReorder 0 0.0
TcpExtTCPTSReorder 0 0.0
Regards,
Vijay
^ permalink raw reply
* Re: [E1000-devel] 82571EB: Detected Hardware Unit Hang
From: Joe Jin @ 2012-11-28 8:31 UTC (permalink / raw)
To: Ben Hutchings
Cc: Fujinaka, Todd, Mary Mcgrath, netdev@vger.kernel.org,
e1000-devel@lists.sf.net, linux-kernel@vger.kernel.org, linux-pci
In-Reply-To: <1354039840.2701.14.camel@bwh-desktop.uk.solarflarecom.com>
On 11/28/12 02:10, Ben Hutchings wrote:
> On Tue, 2012-11-27 at 17:32 +0000, Fujinaka, Todd wrote:
>> Forgive me if I'm being too repetitious as I think some of this has
>> been mentioned in the past.
>>
>> We (and by we I mean the Ethernet part and driver) can only change the
>> advertised availability of a larger MaxPayloadSize. The size is
>> negotiated by both sides of the link when the link is established. The
>> driver should not change the size of the link as it would be poking at
>> registers outside of its scope and is controlled by the upstream
>> bridge (not us).
> [...]
>
> MaxPayloadSize (MPS) is not negotiated between devices but is programmed
> by the system firmware (at least for devices present at boot - the
> kernel may be responsible in case of hotplug). You can use the kernel
> parameter 'pci=pcie_bus_perf' (or one of several others) to set a policy
> that overrides this, but no policy will allow setting MPS above the
> device's MaxPayloadSizeSupported (MPSS).
>
Ben,
Unfortunately I'm using 3.0.x kernel and this is not included in the kernel.
So I'm trying to use ethtool modify it from eeprom to see if help or no.
Todd, I'll review all MaxPayload for all devices, but need to say if it mismatch,
customer could not modify it from BIOS for there was not entry at there, to
test it, we have to find how to verify if this is the root cause, so still
need to find the offset in eeprom.
Thanks in advance,
Joe
^ permalink raw reply
* Re: [PATCH 1/2] smsc75xx: refactor entering suspend modes
From: Steve Glendinning @ 2012-11-28 8:34 UTC (permalink / raw)
To: Alan Stern; +Cc: Bjørn Mork, netdev, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.44L0.1211271443150.1489-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
Hi Alan,
>> udev->do_remote_wakeup is set in choose_wakeup() in
>> drivers/usb/core/driver.c. AFAICS it is always set as long as
>> device_may_wakeup(&udev->dev) is true.
>
> That's right. But is device_may_wakeup(&udev->dev) true?
>
> By default it wouldn't be. The normal way to set it is for the user or
> a program to do:
>
> echo enabled >/sys/bus/usb/devices/.../power/wakeup
>
> Of course, a driver could disregard the user's choice and set the flag
> by itself.
If I set that from userspace the system is able to resume, but I can't
work out how to successfully set this from the driver. I believe the
driver should be overriding this as if the user has asked for the
device to wake on lan they're expecting this to resume the system.
I've tried placing various combinations of device_set_wakeup_capable
and device_set_wakeup_enable in different places (bind, suspend), but
it still doesn't allow the device to resume from suspend. How should
I do this?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: TCP and reordering
From: Vijay Subramanian @ 2012-11-28 8:35 UTC (permalink / raw)
To: Saku Ytti; +Cc: David Miller, rick.jones2, netdev
In-Reply-To: <20121128072611.GA26010@pob.ytti.fi>
>
> My proposal (or question more accurately) was to add 'reorder' counter to
> sockets, which would increment when duplicate ACK is followed by same
> sequence twice.
> Then you could automatically/dynamically delay duplicate acks, as you'd
> start to expect to receive the frames, out-of-order. Giving non-lossy
> reordering links pretty much 100% same performance as non-lossy in-order
> links.
RFC 5681 says that out-of-order packets should be acked immediately.
Please see section 4.2 for detailed reasoning.
It also explains why acks should not be delayed too much.
Also note that reordering is tracked on the sender side using the per
flow variable tp->reordering . This measures the amount of reordering
on the connection so that
fast retransmit and other loss recovery mechanisms are not entered
prematurely. Doesn't this behavior at the sender already provide the
behavior you seek?
Regards,
Vijay
^ permalink raw reply
* Re: TCP and reordering
From: Saku Ytti @ 2012-11-28 8:54 UTC (permalink / raw)
To: netdev
In-Reply-To: <CAGK4HS_GVZqoGVjd87N6skM-SBWyPtY-AhiR9FDcR8ty5x6Xbg@mail.gmail.com>
On (2012-11-28 00:35 -0800), Vijay Subramanian wrote:
> Also note that reordering is tracked on the sender side using the per
> flow variable tp->reordering . This measures the amount of reordering
> on the connection so that
> fast retransmit and other loss recovery mechanisms are not entered
> prematurely. Doesn't this behavior at the sender already provide the
> behavior you seek?
Sorry I don't seem to understand what you mean. Do you mind explaining how
the sender can help to restore performance on reordering network?
--
++ytti
^ permalink raw reply
* Re: TCP and reordering
From: David Woodhouse @ 2012-11-28 9:08 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: David Miller, saku, rick.jones2, netdev
In-Reply-To: <CAGK4HS8=NfcyvcNjC3h1wEjgQFCYoNeuWXj8n5Ruukeg+6j=SQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 804 bytes --]
On Wed, 2012-11-28 at 00:22 -0800, Vijay Subramanian wrote:
>
> I don't believe reordering is tracked on the receiver side but on the
> sender, there are SNMB_MIB items.
> They can be tracked and can be viewed using nstat/netstat
>
> # nstat -az | grep -i reorder
> TcpExtTCPFACKReorder 0 0.0
> TcpExtTCPSACKReorder 0 0.0
> TcpExtTCPRenoReorder 0 0.0
> TcpExtTCPTSReorder 0 0.0
Thanks. For me after a 64MiB download, I have an increase of one FACK,
one SACK and one TS reorder. So my connection probably does even less
reordering than I thought, and thus isn't particularly relevant to this
conversation. I'll shut up now and go back to playing with ATM.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* [PATCH] net: ICMPv6 packets transmitted on wrong interface if nfmark is mangled
From: Dries De Winter @ 2012-11-28 9:09 UTC (permalink / raw)
To: David S. Miller, Pablo Neira Ayuso, Patrick McHardy
Cc: netdev, netfilter-devel
In-Reply-To: <22884633.2468.1354092935228.JavaMail.driesdw@sahwcmp0020>
From: Dries De Winter <dries.dewinter@gmail.com>
The IPv6 mangle table may change the source/destination address and skb->mark
of a packet. Therefore it may be necessary to "reroute" a packet after it
traversed this table. But this should not happen for some special packets like
neighbour solicitations and MLD reports: they have an explicit destination, not
originating from the routing table. Rerouting these packets may cause them to
go out on the wrong interface or not to go out at all depending on the routing
table.
I propose a patch which allows to mark a dst_entry as "non-reroutable".
icmp6_dst_alloc() (used by ndisc and MLD implementation) will always mark the
allocated dst_entry as such. A check is added to netfilter (IPv6-only) so
packets heading for a non-reroutable destination are never rerouted.
Detailed discussion about the patch:
- It is based on 3.6.7.
- Are there other examples of dsts but ICMPv6 that should be non-reroutable?
- Are there other situations but rerouting by netfilter in which this new flag
should be considered?
- Similar logic exists in IPv4 so local multicast/broadcast messages are
potentially transmitted on the wrong interface. However, it's a less likely
corner case there because those packets are treated differently by
local output routing: multicast/broadcast messages are by default routed to
the interface with a matching source IP-address. But this logic is invalid
because (1) it is allowed to send messages with a source IP-address
different from your own and (2) it is allowed to assign the same IP-address
on multiple interfaces. So I feel that also in the case of IPv4 it should
be possible to forbid rerouting for some special packets.
Regards,
Dries De Winter
SoftAtHome
Signed-off-by: Dries De Winter <dries.dewinter@gmail.com>
---
diff --git a/include/net/dst.h b/include/net/dst.h
index 621e351..8b92678 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -61,6 +61,7 @@ struct dst_entry {
#define DST_NOPEER 0x0040
#define DST_FAKE_RTABLE 0x0080
#define DST_XFRM_TUNNEL 0x0100
+#define DST_NOREROUTE 0x0200
unsigned short pending_confirm;
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index db31561..5b98145 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -23,6 +23,10 @@ int ip6_route_me_harder(struct sk_buff *skb)
.saddr = iph->saddr,
};
+ dst = skb_dst(skb);
+ if (dst && (dst->flags & DST_NOREROUTE))
+ return 0;
+
dst = ip6_route_output(net, skb->sk, &fl6);
if (dst->error) {
IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 070a3ce..1c7d377 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1234,7 +1234,7 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
}
}
- rt->dst.flags |= DST_HOST;
+ rt->dst.flags |= DST_HOST | DST_NOREROUTE;
rt->dst.output = ip6_output;
rt->n = neigh;
atomic_set(&rt->dst.__refcnt, 1);
^ permalink raw reply related
* RE: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: David Laight @ 2012-11-28 9:21 UTC (permalink / raw)
To: chas williams - CONTRACTOR, David Woodhouse
Cc: Krzysztof Mazur, davem, netdev, linux-kernel, nathan
In-Reply-To: <20121127135434.0728cd4f@thirdoffive.cmf.nrl.navy.mil>
> On Tue, 27 Nov 2012 18:02:29 +0000
> David Woodhouse <dwmw2@infradead.org> wrote:
>
> > In solos-pci at least, the ops->close() function doesn't flush all
> > pending skbs for this vcc before returning. So can be a tasklet
> > somewhere which has loaded the address of the vcc->pop function from one
> > of them, and is going to call it in some unspecified amount of time.
> >
> > Should we make the device's ->close function wait for all TX and RX skbs
> > for this vcc to complete?
>
> the driver's close routine should wait for any of the pending tx and rx
> to complete. take a look at the he.c in driver/atm
I'm not sure that sleeping for long periods in close() is always a
good idea. If the process is event driven it will be unable to
handle events on other fd until the close completes.
This may be known not to be true in this case, but is more generally
a problem.
In this case the close should probably (IMHO at least) only sleep
while pending tx and rx are aborted/discarded.
Even when it might make sense to sleep in close until tx drains
there needs to be a finite timeout before it become abortive.
David
^ permalink raw reply
* Re: [PATCH v3 8/7] pppoatm: fix missing wakeup in pppoatm_send()
From: David Woodhouse @ 2012-11-28 9:24 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: chas williams - CONTRACTOR, netdev, linux-kernel, davem
In-Reply-To: <20121128081237.GA30488@shrek.podlesie.net>
[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]
On Wed, 2012-11-28 at 09:12 +0100, Krzysztof Mazur wrote:
> On Wed, Nov 28, 2012 at 12:48:17AM +0000, David Woodhouse wrote:
> > On Tue, 2012-11-27 at 10:23 -0500, chas williams - CONTRACTOR wrote:
> > > yes, but dont call it 8/7 since that doesnt make sense.
> >
> > It made enough sense when it was a single patch appended to a thread of
> > 7 other patches from Krzysztof. But now it's all got a little more
> > complex, so I've tried to collect together the latest version of
> > everything we've discussed:
>
> There was also discussion about patch 9/7 "pppoatm: wakeup after ATM
> unlock only when it's needed".
True. Is that really necessary? How often is the lock actually taken? Is
it once per packet that PPP sends (which is mostly just LCP
echo/response during an active connection)? And does that really warrant
the optimisation?
This is a tasklet that we used to run after absolutely *every* packet,
remember. Optimising *that* made sense, but I'm less sure it's worth the
added complexity for this case. As I have a vague recollection that we
decided we couldn't use the existing BLOCKED bit for it... or can we?
Can this work? Feel free to replace that test_bit() and the
corresponding comment, with a test_and_clear_bit() and a new comment
explaining *why* it's safe... while I go make another cup of tea.
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 446a7f0..da58863 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -113,7 +113,13 @@ static void pppoatm_release_cb(struct atm_vcc *atmvcc)
{
struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
- tasklet_schedule(&pvcc->wakeup_tasklet);
+ /*
+ * We can't clear it here because I haven't had enough caffeine
+ * this morning to deal with the concurrency issues. Just leave
+ * it set, and let pppoatm_pop() clear it later.
+ */
+ if (test_bit(BLOCKED, &pvcc->blocked))
+ tasklet_schedule(&pvcc->wakeup_tasklet);
if (pvcc->old_release_cb)
pvcc->old_release_cb(atmvcc);
}
@@ -342,6 +348,12 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
bh_unlock_sock(sk_atm(vcc));
return ret;
nospace:
+ /*
+ * Needs to happen (and be flushed, hence test_and_) before we unlock
+ * the socket. It needs to be seen by the time our ->release_cb gets
+ * called.
+ */
+ test_and_set_bit(BLOCKED, &pvcc->blocked);
bh_unlock_sock(sk_atm(vcc));
/*
* We don't have space to send this SKB now, but we might have
> > David Woodhouse (5):
> > atm: Add release_cb() callback to vcc
> > pppoatm: fix missing wakeup in pppoatm_send()
> > br2684: fix module_put() race
>
> for the three patches above:
>
> Acked-by: Krzysztof Mazur <krzysiek@podlesie.net>
Ta.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply related
* Re: [PATCH v6 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
From: Rafael J. Wysocki @ 2012-11-28 9:29 UTC (permalink / raw)
To: Ming Lei
Cc: linux-pm, linux-kernel, Alan Stern, Oliver Neukum, Minchan Kim,
Greg Kroah-Hartman, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-mm
In-Reply-To: <CACVXFVODD9fRqQc3kR58OJm3ERgBWojnx=790xGwu=MPGaSmMA@mail.gmail.com>
On Wednesday, November 28, 2012 12:34:36 PM Ming Lei wrote:
> On Wed, Nov 28, 2012 at 5:19 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >
> > Please use counters instead of walking the whole path every time. Ie. in
> > addition to the flag add a counter to store the number of the device's
> > children having that flag set.
>
> Even though counter is added, walking the whole path can't be avoided too,
> and may be a explicit walking or recursion, because pm_runtime_set_memalloc_noio
> is required to set or clear the flag(or increase/decrease the counter) of
> devices in the whole path.
But it doesn't have to walk the children. Moreover, with counters it only
needs to walk the whole path if all devices in it need to be updated. For
example, if you call pm_runtime_set_memalloc_noio(dev, true) for a device
whose parent's counter is greater than zero already, you don't need to
walk the path above the parent.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/2] smsc75xx: refactor entering suspend modes
From: Bjørn Mork @ 2012-11-28 9:31 UTC (permalink / raw)
To: Steve Glendinning; +Cc: Alan Stern, netdev, linux-usb
In-Reply-To: <CAKh2mn7+0itrJNxmh6Wv9njGaNOY48N_zfJm6=gTeMoF2VzW4g@mail.gmail.com>
Steve Glendinning <steve@shawell.net> writes:
>>> udev->do_remote_wakeup is set in choose_wakeup() in
>>> drivers/usb/core/driver.c. AFAICS it is always set as long as
>>> device_may_wakeup(&udev->dev) is true.
>>
>> That's right. But is device_may_wakeup(&udev->dev) true?
>>
>> By default it wouldn't be. The normal way to set it is for the user or
>> a program to do:
>>
>> echo enabled >/sys/bus/usb/devices/.../power/wakeup
>>
>> Of course, a driver could disregard the user's choice and set the flag
>> by itself.
>
> If I set that from userspace the system is able to resume, but I can't
> work out how to successfully set this from the driver. I believe the
> driver should be overriding this as if the user has asked for the
> device to wake on lan they're expecting this to resume the system.
>
> I've tried placing various combinations of device_set_wakeup_capable
> and device_set_wakeup_enable in different places (bind, suspend), but
> it still doesn't allow the device to resume from suspend. How should
> I do this?
I may be completely wrong here, but this is how I believe it is supposed
to work... The device can be suspended for two possible reasons:
1) system suspend. If the user want the device to wake the system, then
(s)he will do
echo enabled >/sys/bus/usb/devices/.../power/wakeup
If this isn't set, then there is no reason for the driver to request
remote wakeup while the system is suspended.
2) autosuspend. Any interface driver needing remote wakeup will set
intf->needs_remote_wakeup, which makes autosuspend_check() set
udev->do_remote_wakeup
If all my guesses and assumptions are right, then you want to set
intf->needs_remote_wakeup unconditionally. This will make the USB core
enable remote wakeup on autosuspend.
Remote wakeup will not be enabled on system suspend unless the user (or
a userspace program on the users behalf) has requested it.
Bjørn
^ permalink raw reply
* RE: Re: RTL 8169 linux driver question
From: David Laight @ 2012-11-28 9:33 UTC (permalink / raw)
To: Francois Romieu; +Cc: Stéphane ANCELOT, netdev, sancelot, Hayes Wang
In-Reply-To: <20121127224605.GA10228@electric-eye.fr.zoreil.com>
> +static struct rtl_coalesce_scale *rtl_coalesce_scale(struct net_device *dev)
> +{
...
> +}
> +
> +static int rtl_get_coalesce(struct net_device *dev, struct ethtool_coalesce *ec)
> +{
...
> +}
> +
> +static int rtl_set_coalesce(struct net_device *dev, struct ethtool_coalesce *ec)
> +{
...
> +}
Those functions are horrid - so horrid I've deleted the contents.
David
^ permalink raw reply
* Re: [PATCH v6 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
From: Ming Lei @ 2012-11-28 9:47 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-pm, linux-kernel, Alan Stern, Oliver Neukum, Minchan Kim,
Greg Kroah-Hartman, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-mm
In-Reply-To: <1408044.6czCGhbHJH@vostro.rjw.lan>
On Wed, Nov 28, 2012 at 5:29 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>
> But it doesn't have to walk the children. Moreover, with counters it only
Yeah, I got it, it is the advantage of counter, but with extra 'int'
field introduced
in 'struct device'.
> needs to walk the whole path if all devices in it need to be updated. For
> example, if you call pm_runtime_set_memalloc_noio(dev, true) for a device
> whose parent's counter is greater than zero already, you don't need to
> walk the path above the parent.
We still can do it with the flag only, pm_runtime_set_memalloc_noio(dev, true)
can return immediately if one parent or the 'dev' flag is true.
But considered that the pm_runtime_set_memalloc_noio(dev, false) is only
called in a very infrequent path(network/block device->remove()), looks the
introduced cost isn't worthy of the obtained advantage.
So could you accept not introducing counter? and I will update with the
above improvement you suggested.
Thanks,
--
Ming Lei
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] bonding: rlb mode of bond should not alter ARP originating via bridge
From: Zheng Li @ 2012-11-28 9:57 UTC (permalink / raw)
To: netdev, fubar, andy; +Cc: linux-kernel, davem, joe.jin, zheng.x.li
Do not modify or load balance ARP packets passing through balance-alb
mode (wherein the ARP did not originate locally, and arrived via a bridge).
Modifying pass-through ARP replies causes an incorrect MAC address
to be placed into the ARP packet, rendering peers unable to communicate
with the actual destination from which the ARP reply originated.
Load balancing pass-through ARP requests causes an entry to be
created for the peer in the rlb table, and bond_alb_monitor will
occasionally issue ARP updates to all peers in the table instrucing them
as to which MAC address they should communicate with; this occurs when
some event sets rx_ntt. In the bridged case, however, the MAC address
used for the update would be the MAC of the slave, not the actual source
MAC of the originating destination. This would render peers unable to
communicate with the destinations beyond the bridge.
Signed-off-by: Zheng Li <zheng.x.li@oracle.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Cc: "David S. Miller" <davem@davemloft.net>
---
drivers/net/bonding/bond_alb.c | 6 ++++++
drivers/net/bonding/bonding.h | 13 +++++++++++++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e15cc11..6fecb52 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -694,6 +694,12 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond)
struct arp_pkt *arp = arp_pkt(skb);
struct slave *tx_slave = NULL;
+ /* Don't modify or load balance ARPs that do not originate locally
+ * (e.g.,arrive via a bridge).
+ */
+ if (!bond_slave_has_mac(bond, arp->mac_src))
+ return NULL;
+
if (arp->op_code == htons(ARPOP_REPLY)) {
/* the arp must be sent on the selected
* rx channel
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index f8af2fc..6dded56 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -22,6 +22,7 @@
#include <linux/in6.h>
#include <linux/netpoll.h>
#include <linux/inetdevice.h>
+#include <linux/etherdevice.h>
#include "bond_3ad.h"
#include "bond_alb.h"
@@ -450,6 +451,18 @@ static inline void bond_destroy_proc_dir(struct bond_net *bn)
}
#endif
+static inline struct slave *bond_slave_has_mac(struct bonding *bond,
+ const u8 *mac)
+{
+ int i = 0;
+ struct slave *tmp;
+
+ bond_for_each_slave(bond, tmp, i)
+ if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
+ return tmp;
+
+ return NULL;
+}
/* exported from bond_main.c */
extern int bond_net_id;
--
1.7.6.5
^ permalink raw reply related
* Re: [PATCH v3 8/7] pppoatm: fix missing wakeup in pppoatm_send()
From: Krzysztof Mazur @ 2012-11-28 9:58 UTC (permalink / raw)
To: David Woodhouse; +Cc: chas williams - CONTRACTOR, netdev, linux-kernel, davem
In-Reply-To: <1354094649.21562.34.camel@shinybook.infradead.org>
On Wed, Nov 28, 2012 at 09:24:09AM +0000, David Woodhouse wrote:
> On Wed, 2012-11-28 at 09:12 +0100, Krzysztof Mazur wrote:
> > On Wed, Nov 28, 2012 at 12:48:17AM +0000, David Woodhouse wrote:
> > > On Tue, 2012-11-27 at 10:23 -0500, chas williams - CONTRACTOR wrote:
> > > > yes, but dont call it 8/7 since that doesnt make sense.
> > >
> > > It made enough sense when it was a single patch appended to a thread of
> > > 7 other patches from Krzysztof. But now it's all got a little more
> > > complex, so I've tried to collect together the latest version of
> > > everything we've discussed:
> >
> > There was also discussion about patch 9/7 "pppoatm: wakeup after ATM
> > unlock only when it's needed".
>
> True. Is that really necessary? How often is the lock actually taken? Is
> it once per packet that PPP sends (which is mostly just LCP
> echo/response during an active connection)? And does that really warrant
> the optimisation?
>
> This is a tasklet that we used to run after absolutely *every* packet,
> remember. Optimising *that* made sense, but I'm less sure it's worth the
> added complexity for this case. As I have a vague recollection that we
> decided we couldn't use the existing BLOCKED bit for it... or can we?
>
> Can this work? Feel free to replace that test_bit() and the
> corresponding comment, with a test_and_clear_bit() and a new comment
> explaining *why* it's safe... while I go make another cup of tea.
>
ok, I think that we should just drop that patch, with test_bit()
I think it's no longer an optimization.
Krzysiek
^ permalink raw reply
* Re: [PATCH] br2684: don't send frames on not-ready vcc
From: David Woodhouse @ 2012-11-28 9:58 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: chas williams - CONTRACTOR, davem, netdev, linux-kernel, nathan
In-Reply-To: <20121128080804.GA23175@shrek.podlesie.net>
[-- Attachment #1: Type: text/plain, Size: 624 bytes --]
On Wed, 2012-11-28 at 09:08 +0100, Krzysztof Mazur wrote:
>
> I think you might need also an equivalent of
> "[PATCH v3 3/7] pppoatm: allow assign only on a connected socket".
>
> I'm not sure yet. In will test if I can trigger that Oops on pppoatm
> without that patch. Testing vcc flags might be sufficient - that's
> what I did in the first patch, but you asked what about SOCK_CONNECTED,
> and I think it was really needed.
Even if the READY check avoids the oops, I think the patch makes sense.
Especially as it makes things consistent with pppoatm. I've applied it
to the tree. Thanks.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: Krzysztof Mazur @ 2012-11-28 10:04 UTC (permalink / raw)
To: David Laight
Cc: chas williams - CONTRACTOR, David Woodhouse, davem, netdev,
linux-kernel, nathan
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B70C9@saturn3.aculab.com>
On Wed, Nov 28, 2012 at 09:21:37AM -0000, David Laight wrote:
> > On Tue, 27 Nov 2012 18:02:29 +0000
> > David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > > In solos-pci at least, the ops->close() function doesn't flush all
> > > pending skbs for this vcc before returning. So can be a tasklet
> > > somewhere which has loaded the address of the vcc->pop function from one
> > > of them, and is going to call it in some unspecified amount of time.
> > >
> > > Should we make the device's ->close function wait for all TX and RX skbs
> > > for this vcc to complete?
> >
> > the driver's close routine should wait for any of the pending tx and rx
> > to complete. take a look at the he.c in driver/atm
>
> I'm not sure that sleeping for long periods in close() is always a
> good idea. If the process is event driven it will be unable to
> handle events on other fd until the close completes.
> This may be known not to be true in this case, but is more generally
> a problem.
> In this case the close should probably (IMHO at least) only sleep
> while pending tx and rx are aborted/discarded.
>
> Even when it might make sense to sleep in close until tx drains
> there needs to be a finite timeout before it become abortive.
>
The ->close() routine can just abort any pending rx/tx and just wait
for completion of currently running rx/tx code. That shouldn't take
long.
Krzysiek
^ permalink raw reply
* Re: [PATCH v6 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
From: Rafael J. Wysocki @ 2012-11-28 10:06 UTC (permalink / raw)
To: Ming Lei
Cc: linux-pm, linux-kernel, Alan Stern, Oliver Neukum, Minchan Kim,
Greg Kroah-Hartman, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-mm
In-Reply-To: <CACVXFVP=3s3pawyEbogjb=PfbSeD1B+LFk7g04FAMkGuXDQUbQ@mail.gmail.com>
On Wednesday, November 28, 2012 11:57:19 AM Ming Lei wrote:
> On Wed, Nov 28, 2012 at 5:19 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday, November 24, 2012 08:59:14 PM Ming Lei wrote:
> >> The patch introduces the flag of memalloc_noio in 'struct dev_pm_info'
> >> to help PM core to teach mm not allocating memory with GFP_KERNEL
> >> flag for avoiding probable deadlock.
> >>
> >> As explained in the comment, any GFP_KERNEL allocation inside
> >> runtime_resume() or runtime_suspend() on any one of device in
> >> the path from one block or network device to the root device
> >> in the device tree may cause deadlock, the introduced
> >> pm_runtime_set_memalloc_noio() sets or clears the flag on
> >> device in the path recursively.
> >>
> >> Cc: Alan Stern <stern@rowland.harvard.edu>
> >> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> >> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> >> ---
> >> v5:
> >> - fix code style error
> >> - add comment on clear the device memalloc_noio flag
> >> v4:
> >> - rename memalloc_noio_resume as memalloc_noio
> >> - remove pm_runtime_get_memalloc_noio()
> >> - add comments on pm_runtime_set_memalloc_noio
> >> v3:
> >> - introduce pm_runtime_get_memalloc_noio()
> >> - hold one global lock on pm_runtime_set_memalloc_noio
> >> - hold device power lock when accessing memalloc_noio_resume
> >> flag suggested by Alan Stern
> >> - implement pm_runtime_set_memalloc_noio without recursion
> >> suggested by Alan Stern
> >> v2:
> >> - introduce pm_runtime_set_memalloc_noio()
> >> ---
> >> drivers/base/power/runtime.c | 60 ++++++++++++++++++++++++++++++++++++++++++
> >> include/linux/pm.h | 1 +
> >> include/linux/pm_runtime.h | 3 +++
> >> 3 files changed, 64 insertions(+)
> >>
> >> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> >> index 3148b10..3e198a0 100644
> >> --- a/drivers/base/power/runtime.c
> >> +++ b/drivers/base/power/runtime.c
> >> @@ -124,6 +124,66 @@ unsigned long pm_runtime_autosuspend_expiration(struct device *dev)
> >> }
> >> EXPORT_SYMBOL_GPL(pm_runtime_autosuspend_expiration);
> >>
> >> +static int dev_memalloc_noio(struct device *dev, void *data)
> >> +{
> >> + return dev->power.memalloc_noio;
> >> +}
> >> +
> >> +/*
> >> + * pm_runtime_set_memalloc_noio - Set a device's memalloc_noio flag.
> >> + * @dev: Device to handle.
> >> + * @enable: True for setting the flag and False for clearing the flag.
> >> + *
> >> + * Set the flag for all devices in the path from the device to the
> >> + * root device in the device tree if @enable is true, otherwise clear
> >> + * the flag for devices in the path whose siblings don't set the flag.
> >> + *
> >
> > Please use counters instead of walking the whole path every time. Ie. in
> > addition to the flag add a counter to store the number of the device's
> > children having that flag set.
>
> Thanks for your review.
>
> IMO, pm_runtime_set_memalloc_noio() is only called in
> probe() and release() of block device and network device, which is
> in a very infrequent path, so I am wondering if it is worthy of introducing
> another counter for all devices.
Well, it may be unfrequent, but does it mean it has to do things that may
be avoided (ie. walking the children of every node in the path in some cases)?
I don't really think that the counters would cost us that much anyway.
> Also looks the current implementation of pm_runtime_set_memalloc_noio()
> is simple and clean enough with the flag, IMO.
I know you always know better. :-)
> > I would use the flag only to store the information that
> > pm_runtime_set_memalloc_noio(dev, true) has been run for this device directly
> > and I'd use a counter for everything else.
> >
> > That is, have power.memalloc_count that would be incremented when (1)
> > pm_runtime_set_memalloc_noio(dev, true) is called for that device and (2) when
> > power.memalloc_count for one of its children changes from 0 to 1 (and
> > analogously for decrementation). Then, check the counter in rpm_callback().
>
> Sorry, could you explain in a bit detail why we need the counter? Looks only
> checking the flag in rpm_callback() is enough, doesn't it?
Why would I want to use power.memalloc_count in addition to the
power.memalloc_noio flag?
Consider this:
pm_runtime_set_memalloc_noio(dev):
return if power.memalloc_noio is set
set power.memalloc_noio
loop:
increment power.memalloc_count
if power.memalloc_count is 1 now switch to parent and go to loop
pm_runtime_clear_memalloc_noio(dev):
return if power.memalloc_noio is unset
unset power.memalloc_noio
loop:
decrement power.memalloc_count
if power.memalloc_count is 0 now switch to parent and go to loop
Looks kind of simpler, doesn't it?
And why rpm_callback() should check power.memalloc_count instead of the count?
Because power.memalloc_noio will only be set for devices that
pm_runtime_set_memalloc_noio(dev) was called for directly (not necessarily for
the parents).
And that works even if someone calls any of them twice in a row for the same
device (presumably by mistake) and doesn't have to make any assumptions
about devices it is called for.
> > Besides, don't you need to check children for the arg device itself?
>
> It isn't needed since the children of network/block device can't be
> involved of the deadlock in runtime PM path.
>
> Also, the function is only called by network device or block device
> subsystem, both the two kind of device are class device and should
> have no children.
OK, so not walking the arg device's children is an optimization related to
some assumptions regarding who's supposed to use this routine. That should
be clearly documented.
However, I'd prefer it not to make such assumptions in the first place.
> >> + * The function should only be called by block device, or network
> >> + * device driver for solving the deadlock problem during runtime
> >> + * resume/suspend:
> >> + *
> >> + * If memory allocation with GFP_KERNEL is called inside runtime
> >> + * resume/suspend callback of any one of its ancestors(or the
> >> + * block device itself), the deadlock may be triggered inside the
> >> + * memory allocation since it might not complete until the block
> >> + * device becomes active and the involed page I/O finishes. The
> >> + * situation is pointed out first by Alan Stern. Network device
> >> + * are involved in iSCSI kind of situation.
> >> + *
> >> + * The lock of dev_hotplug_mutex is held in the function for handling
> >> + * hotplug race because pm_runtime_set_memalloc_noio() may be called
> >> + * in async probe().
> >> + *
> >> + * The function should be called between device_add() and device_del()
> >> + * on the affected device(block/network device).
> >> + */
> >> +void pm_runtime_set_memalloc_noio(struct device *dev, bool enable)
> >> +{
> >> + static DEFINE_MUTEX(dev_hotplug_mutex);
> >
> > What's the mutex for?
>
> It is for avoiding hotplug race, for example, without the mutex,
> another child may set the flag between the time device_for_each_child()
> runs and the next loop iteration in pm_runtime_set_memalloc_noio(false).
OK
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [RFC PATCH] tcp: introduce raw access to experimental options
From: Einar Lueck @ 2012-11-28 10:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, frankbla, raspl, ubacher, samudrala
In-Reply-To: <50ACE496.20705@linux.vnet.ibm.com>
On 11/21/2012 03:26 PM, Einar Lueck wrote:
> On 11/16/2012 07:44 PM, David Miller wrote:
>>
>> Unprivileged access to set and fetch these things? I don't think
>> that's a good idea.
>>
>> Also, your code has a lot of coding style errors.
>>
>
> Would a restriction to a CAP_NET_ADMIN like privilege level address and
> resolve your concerns?
>
> Thx,
> Einar.
Would CAP_NET_ADMIN address your concerns? If would go ahead then and
work on a corresponding new version of the patch.
Thx,
Einar.
^ permalink raw reply
* Re: [PATCH v6 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
From: Rafael J. Wysocki @ 2012-11-28 10:15 UTC (permalink / raw)
To: Ming Lei
Cc: linux-pm, linux-kernel, Alan Stern, Oliver Neukum, Minchan Kim,
Greg Kroah-Hartman, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-mm
In-Reply-To: <CACVXFVOk45wr8jv3w=KO7uTThGSTSkq0FRsPD6p_AyQZLWGQJg@mail.gmail.com>
On Wednesday, November 28, 2012 05:47:18 PM Ming Lei wrote:
> On Wed, Nov 28, 2012 at 5:29 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >
> > But it doesn't have to walk the children. Moreover, with counters it only
>
> Yeah, I got it, it is the advantage of counter, but with extra 'int'
> field introduced
> in 'struct device'.
>
> > needs to walk the whole path if all devices in it need to be updated. For
> > example, if you call pm_runtime_set_memalloc_noio(dev, true) for a device
> > whose parent's counter is greater than zero already, you don't need to
> > walk the path above the parent.
>
> We still can do it with the flag only, pm_runtime_set_memalloc_noio(dev, true)
> can return immediately if one parent or the 'dev' flag is true.
>
> But considered that the pm_runtime_set_memalloc_noio(dev, false) is only
> called in a very infrequent path(network/block device->remove()), looks the
> introduced cost isn't worthy of the obtained advantage.
>
> So could you accept not introducing counter? and I will update with the
> above improvement you suggested.
Well, please see my other message I sent a while ago. :-)
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH 1/2] smsc75xx: refactor entering suspend modes
From: Steve Glendinning @ 2012-11-28 10:16 UTC (permalink / raw)
To: Bjørn Mork; +Cc: Alan Stern, netdev, linux-usb, David Miller
In-Reply-To: <874nka3wgx.fsf@nemi.mork.no>
Hi Bjorn,
On 28 November 2012 09:31, Bjørn Mork <bjorn@mork.no> wrote:
>
> Remote wakeup will not be enabled on system suspend unless the user (or
> a userspace program on the users behalf) has requested it.
If a user types "ethtool -s eth2 wol p" they *are* explicitly
requesting the ethernet device to bring the system out of suspend, so
I think the ethernet driver should set the feature automatically.
from drivers/base/power/wakeup.c:
* By default, most devices should leave wakeup disabled. The exceptions are
* devices that everyone expects to be wakeup sources: keyboards, power buttons,
* possibly network interfaces, etc.
Steve
^ permalink raw reply
* Re: [PATCH v3 8/7] pppoatm: fix missing wakeup in pppoatm_send()
From: David Woodhouse @ 2012-11-28 10:19 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: chas williams - CONTRACTOR, netdev, linux-kernel, davem
In-Reply-To: <20121128095843.GA8974@shrek.podlesie.net>
[-- Attachment #1: Type: text/plain, Size: 2560 bytes --]
On Wed, 2012-11-28 at 10:58 +0100, Krzysztof Mazur wrote:
> ok, I think that we should just drop that patch, with test_bit()
> I think it's no longer an optimization.
After another cup of tea, it now uses test_and_clear_bit()... and
doesn't break the carefully crafted handling of the BLOCKED bit in the
normal flow control case, by setting it at the nospace: label!
From b2cf6a466697ecf19061cb11b8f4ec5bb381550a Mon Sep 17 00:00:00 2001
From: David Woodhouse <David.Woodhouse@intel.com>
Date: Wed, 28 Nov 2012 10:15:05 +0000
Subject: [PATCH] pppoatm: optimise PPP channel wakeups after sock_owned_by_user()
We don't need to schedule the wakeup tasklet on *every* unlock; only if we
actually blocked the channel in the first place.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
net/atm/pppoatm.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 446a7f0..172e44e 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -113,7 +113,17 @@ static void pppoatm_release_cb(struct atm_vcc *atmvcc)
{
struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
- tasklet_schedule(&pvcc->wakeup_tasklet);
+ /*
+ * As in pppoatm_pop(), it's safe to clear the BLOCKED bit here because
+ * the wakeup *can't* race with pppoatm_send(). They both hold the PPP
+ * channel's ->downl lock. And the potential race with *setting* it,
+ * which leads to the double-check dance in pppoatm_may_send(), doesn't
+ * exist here. In the sock_owned_by_user() case in pppoatm_send(), we
+ * set the BLOCKED bit while the socket is still locked. We know that
+ * ->release_cb() can't be called until that's done.
+ */
+ if (test_and_clear_bit(BLOCKED, &pvcc->blocked))
+ tasklet_schedule(&pvcc->wakeup_tasklet);
if (pvcc->old_release_cb)
pvcc->old_release_cb(atmvcc);
}
@@ -292,8 +302,15 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
vcc = ATM_SKB(skb)->vcc;
bh_lock_sock(sk_atm(vcc));
- if (sock_owned_by_user(sk_atm(vcc)))
+ if (sock_owned_by_user(sk_atm(vcc))) {
+ /*
+ * Needs to happen (and be flushed, hence test_and_) before we unlock
+ * the socket. It needs to be seen by the time our ->release_cb gets
+ * called.
+ */
+ test_and_set_bit(BLOCKED, &pvcc->blocked);
goto nospace;
+ }
if (test_bit(ATM_VF_RELEASED, &vcc->flags)
|| test_bit(ATM_VF_CLOSE, &vcc->flags)
|| !test_bit(ATM_VF_READY, &vcc->flags)) {
--
1.8.0
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply related
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