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 i
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 w
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 Krz
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 collec
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
> > things
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
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 pppo
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
> ne
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
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
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 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 optimis
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 h
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.
>
Loo
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 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 w
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 morni
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 w
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 (a
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 *s
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
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.
>
> Fi
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
already
25 matches
Mail list logo
