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
On Wed, Nov 28, 2012 at 09:24:09AM +, David Woodhouse wrote:
> On Wed, 2012-11-28 at 09:12 +0100, Krzysztof Mazur wrote:
> > On Wed, Nov 28, 2012 at 12:48:17AM +, David Woodhouse wrote:
> > > On Tue, 2012-11-27 at 10:23 -0500, chas williams - CONTRACTOR wrote:
> > > > yes, but dont call it
On Wed, 2012-11-28 at 09:12 +0100, Krzysztof Mazur wrote:
> On Wed, Nov 28, 2012 at 12:48:17AM +, 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
On Wed, Nov 28, 2012 at 12:48:17AM +, 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
On Wed, Nov 28, 2012 at 12:48:17AM +, 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
On Wed, 2012-11-28 at 09:12 +0100, Krzysztof Mazur wrote:
On Wed, Nov 28, 2012 at 12:48:17AM +, 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
On Wed, Nov 28, 2012 at 09:24:09AM +, David Woodhouse wrote:
On Wed, 2012-11-28 at 09:12 +0100, Krzysztof Mazur wrote:
On Wed, Nov 28, 2012 at 12:48:17AM +, David Woodhouse wrote:
On Tue, 2012-11-27 at 10:23 -0500, chas williams - CONTRACTOR wrote:
yes, but dont call it 8/7 since
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
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
On Tue, 27 Nov 2012 13:27:47 +
David Woodhouse wrote:
> > i really would prefer not to use a strange name since it might confuse
> > larger group of people who are more familiar with the traditional meaning
> > of this function. vcc_release() isnt exported so we could rename it if
> >
On Sun, 2012-11-11 at 17:57 -0500, Chas Williams (CONTRACTOR) wrote:
> In message <1352667081.9449.135.ca...@shinybook.infradead.org>,David
> Woodhouse writes:
> >Acked-by: David Woodhouse for your new
> >version of patch #6 (returning DROP_PACKET for !VF_READY), and your
> >followup to my patch
On Sun, 2012-11-11 at 17:57 -0500, Chas Williams (CONTRACTOR) wrote:
In message 1352667081.9449.135.ca...@shinybook.infradead.org,David
Woodhouse writes:
Acked-by: David Woodhouse david.woodho...@intel.com for your new
version of patch #6 (returning DROP_PACKET for !VF_READY), and your
On Tue, 27 Nov 2012 13:27:47 +
David Woodhouse dw...@infradead.org wrote:
i really would prefer not to use a strange name since it might confuse
larger group of people who are more familiar with the traditional meaning
of this function. vcc_release() isnt exported so we could rename it
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
In message <1352667081.9449.135.ca...@shinybook.infradead.org>,David Woodhouse
writes:
>Acked-by: David Woodhouse for your new
>version of patch #6 (returning DROP_PACKET for !VF_READY), and your
>followup to my patch #8, adding the 'need_wakeup' flag. Which we might
>as well merge into (the
In message <2012110437.ga25...@shrek.podlesie.net>,Krzysztof Mazur writes:
>Any race with testing vcc flags is probably not really important
>because:
> - vcc_release_async() does not take any lock so this check
> will be always racy so we are probably allowed to send
>
On Sun, 2012-11-11 at 19:49 +0100, Krzysztof Mazur wrote:
>
> In pppoatm_devppp_ioctl() we also don't have sk->sk_lock.slock lock.
> In original patch synchronization was trivial because callback
> from socket lock is used.
>
> I also though about sharing word with encaps enum - encaps needs
On Sun, Nov 11, 2012 at 05:03:21PM +, David Woodhouse wrote:
> On Sun, 2012-11-11 at 17:12 +0100, Krzysztof Mazur wrote:
> > It would require using atomic ops because also pppoatm_pop() can
> > modify this word. I think it's better to add additional word instead
> > of using atomic ops.
>
>
On Sun, 2012-11-11 at 17:12 +0100, Krzysztof Mazur wrote:
> It would require using atomic ops because also pppoatm_pop() can
> modify this word. I think it's better to add additional word instead
> of using atomic ops.
Or use the existing flags word, perhaps. Only one bit of which is
actually
On Sun, Nov 11, 2012 at 03:26:41PM +, David Woodhouse wrote:
> On Sun, 2012-11-11 at 14:50 +0100, Krzysztof Mazur wrote:
> > Looks and works ok after:
> > + atmvcc->unlock_cb = pppoatm_unlock_cb;
>
> Heh, yeah. That would probably help :)
>
> Not sure if it's really necessary to
On Sun, 2012-11-11 at 14:50 +0100, Krzysztof Mazur wrote:
> Looks and works ok after:
> + atmvcc->unlock_cb = pppoatm_unlock_cb;
Heh, yeah. That would probably help :)
Not sure if it's really necessary to optimise out the unneeded wakeups —
I don't think that code path gets exercised very
On Sun, Nov 11, 2012 at 11:39:53AM +, David Woodhouse wrote:
>
> Right. Something like this then, instead of my previous patch 8/7?
>
> Only addresses the sock_owned_by_user() case and not ATM_VF_RELEASED,
> ATM_VF_CLOSE or !ATM_VF_READY, but your amended patch 6 fixes that I
> think.
>
On Sun, 2012-11-11 at 12:04 +0100, Krzysztof Mazur wrote:
> Yes, but socket can be also locked for a long time, vcc_sendmsg() sleeps
> owning socket lock waiting for memory or atm_may_send().
Right. Something like this then, instead of my previous patch 8/7?
Only addresses the
On Sun, Nov 11, 2012 at 07:28:53AM +, David Woodhouse wrote:
> On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
> > With this tasklet_schedule() we implement a "spin_lock" here, but in
> > this case both conditions (vcc not ready and socket locked) can be
> > true for a long time and
On Sun, Nov 11, 2012 at 07:28:53AM +, David Woodhouse wrote:
On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
With this tasklet_schedule() we implement a spin_lock here, but in
this case both conditions (vcc not ready and socket locked) can be
true for a long time and we can
On Sun, 2012-11-11 at 12:04 +0100, Krzysztof Mazur wrote:
Yes, but socket can be also locked for a long time, vcc_sendmsg() sleeps
owning socket lock waiting for memory or atm_may_send().
Right. Something like this then, instead of my previous patch 8/7?
Only addresses the sock_owned_by_user()
On Sun, Nov 11, 2012 at 11:39:53AM +, David Woodhouse wrote:
Right. Something like this then, instead of my previous patch 8/7?
Only addresses the sock_owned_by_user() case and not ATM_VF_RELEASED,
ATM_VF_CLOSE or !ATM_VF_READY, but your amended patch 6 fixes that I
think.
Looks and
On Sun, 2012-11-11 at 14:50 +0100, Krzysztof Mazur wrote:
Looks and works ok after:
+ atmvcc-unlock_cb = pppoatm_unlock_cb;
Heh, yeah. That would probably help :)
Not sure if it's really necessary to optimise out the unneeded wakeups —
I don't think that code path gets exercised very
On Sun, Nov 11, 2012 at 03:26:41PM +, David Woodhouse wrote:
On Sun, 2012-11-11 at 14:50 +0100, Krzysztof Mazur wrote:
Looks and works ok after:
+ atmvcc-unlock_cb = pppoatm_unlock_cb;
Heh, yeah. That would probably help :)
Not sure if it's really necessary to optimise out the
On Sun, 2012-11-11 at 17:12 +0100, Krzysztof Mazur wrote:
It would require using atomic ops because also pppoatm_pop() can
modify this word. I think it's better to add additional word instead
of using atomic ops.
Or use the existing flags word, perhaps. Only one bit of which is
actually used
On Sun, Nov 11, 2012 at 05:03:21PM +, David Woodhouse wrote:
On Sun, 2012-11-11 at 17:12 +0100, Krzysztof Mazur wrote:
It would require using atomic ops because also pppoatm_pop() can
modify this word. I think it's better to add additional word instead
of using atomic ops.
Or use the
On Sun, 2012-11-11 at 19:49 +0100, Krzysztof Mazur wrote:
In pppoatm_devppp_ioctl() we also don't have sk-sk_lock.slock lock.
In original patch synchronization was trivial because callback
from socket lock is used.
I also though about sharing word with encaps enum - encaps needs only 2
In message 2012110437.ga25...@shrek.podlesie.net,Krzysztof Mazur writes:
Any race with testing vcc flags is probably not really important
because:
- vcc_release_async() does not take any lock so this check
will be always racy so we are probably allowed to send
new
In message 1352667081.9449.135.ca...@shinybook.infradead.org,David Woodhouse
writes:
Acked-by: David Woodhouse david.woodho...@intel.com for your new
version of patch #6 (returning DROP_PACKET for !VF_READY), and your
followup to my patch #8, adding the 'need_wakeup' flag. Which we might
as well
On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
> With this tasklet_schedule() we implement a "spin_lock" here, but in
> this case both conditions (vcc not ready and socket locked) can be
> true for a long time and we can spin here for a long time.
Reading this more carefully this
On Sat, Nov 10, 2012 at 09:02:02PM +, David Woodhouse wrote:
> On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
> > With this tasklet_schedule() we implement a "spin_lock" here, but in
> > this case both conditions (vcc not ready and socket locked) can be
> > true for a long time and
On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
> With this tasklet_schedule() we implement a "spin_lock" here, but in
> this case both conditions (vcc not ready and socket locked) can be
> true for a long time and we can spin here for a long time. I confirmed
> it by reverting patch 1
On Wed, Nov 07, 2012 at 12:52:14PM +, David Woodhouse wrote:
>
> diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
> index 7507c20..56ad541 100644
> --- a/net/atm/pppoatm.c
> +++ b/net/atm/pppoatm.c
> @@ -283,11 +283,11 @@ static int pppoatm_send(struct ppp_channel *chan,
> struct sk_buff
From: David Woodhouse
Date: Sat, 10 Nov 2012 07:36:13 +
> I was hoping for an ack from Chas and/or Krzysztof, especially as I
> hadn't tested my patch. So hopefully there'll be a v4 series of 8
> patches, including this one... and all from the same person, which
> makes it slightly easier to
From: David Woodhouse dw...@infradead.org
Date: Sat, 10 Nov 2012 07:36:13 +
I was hoping for an ack from Chas and/or Krzysztof, especially as I
hadn't tested my patch. So hopefully there'll be a v4 series of 8
patches, including this one... and all from the same person, which
makes it
On Wed, Nov 07, 2012 at 12:52:14PM +, David Woodhouse wrote:
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 7507c20..56ad541 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -283,11 +283,11 @@ static int pppoatm_send(struct ppp_channel *chan,
struct sk_buff *skb)
On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
With this tasklet_schedule() we implement a spin_lock here, but in
this case both conditions (vcc not ready and socket locked) can be
true for a long time and we can spin here for a long time. I confirmed
it by reverting patch 1 (atm:
On Sat, Nov 10, 2012 at 09:02:02PM +, David Woodhouse wrote:
On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
With this tasklet_schedule() we implement a spin_lock here, but in
this case both conditions (vcc not ready and socket locked) can be
true for a long time and we can
On Sat, 2012-11-10 at 21:23 +0100, Krzysztof Mazur wrote:
With this tasklet_schedule() we implement a spin_lock here, but in
this case both conditions (vcc not ready and socket locked) can be
true for a long time and we can spin here for a long time.
Reading this more carefully this
On Fri, 2012-11-09 at 16:30 -0500, David Miller wrote:
> I don't know what to do with this patch because I don't have any
> context whatsoever.
I sent two replies to Krzysztof's series starting with [PATCH v3 0/7]
in Message-Id: <1352240222-363-1-git-send-email-krzys...@podlesie.net>
The first
From: David Woodhouse
Date: Wed, 07 Nov 2012 12:52:14 +
> Now that we can return zero from pppoatm_send() for reasons *other* than
> the queue being full, that means we can't depend on a subsequent call to
> pppoatm_pop() waking the queue, and we might leave it stalled
> indefinitely.
>
>
From: David Woodhouse dw...@infradead.org
Date: Wed, 07 Nov 2012 12:52:14 +
Now that we can return zero from pppoatm_send() for reasons *other* than
the queue being full, that means we can't depend on a subsequent call to
pppoatm_pop() waking the queue, and we might leave it stalled
On Fri, 2012-11-09 at 16:30 -0500, David Miller wrote:
I don't know what to do with this patch because I don't have any
context whatsoever.
I sent two replies to Krzysztof's series starting with [PATCH v3 0/7]
in Message-Id: 1352240222-363-1-git-send-email-krzys...@podlesie.net
The first was
Now that we can return zero from pppoatm_send() for reasons *other* than
the queue being full, that means we can't depend on a subsequent call to
pppoatm_pop() waking the queue, and we might leave it stalled
indefinitely.
Fix this by immediately scheduling the wakeup tasklet. As documented
Now that we can return zero from pppoatm_send() for reasons *other* than
the queue being full, that means we can't depend on a subsequent call to
pppoatm_pop() waking the queue, and we might leave it stalled
indefinitely.
Fix this by immediately scheduling the wakeup tasklet. As documented
50 matches
Mail list logo
