Patchwork [v2,10/10] af_unix: use freezable blocking calls in read

login
register
mail settings
Submitter Colin Cross
Date May 2, 2013, 1:35 a.m.
Message ID <1367458508-9133-11-git-send-email-ccross@android.com>
Download mbox | patch
Permalink /patch/240870/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Colin Cross - May 2, 2013, 1:35 a.m.
Avoid waking up every thread sleeping in read call on an AF_UNIX
socket during suspend and resume by calling a freezable blocking
call.  Previous patches modified the freezer to avoid sending
wakeups to threads that are blocked in freezable blocking calls.

This call was selected to be converted to a freezable call because
it doesn't hold any locks or release any resources when interrupted
that might be needed by another freezing task or a kernel driver
during suspend, and is a common site where idle userspace tasks are
blocked.

Signed-off-by: Colin Cross <ccross@android.com>
---
 net/unix/af_unix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Tejun Heo - May 3, 2013, 12:08 a.m.
On Wed, May 01, 2013 at 06:35:08PM -0700, Colin Cross wrote:
> Avoid waking up every thread sleeping in read call on an AF_UNIX
> socket during suspend and resume by calling a freezable blocking
> call.  Previous patches modified the freezer to avoid sending
> wakeups to threads that are blocked in freezable blocking calls.
> 
> This call was selected to be converted to a freezable call because
> it doesn't hold any locks or release any resources when interrupted
> that might be needed by another freezing task or a kernel driver
> during suspend, and is a common site where idle userspace tasks are
> blocked.

Heh, so you are aware of the deadlock possibilities.  Good selection
of spots.  For all the conversion patches.

 Acked-by: Tejun Heo <tj@kernel.org>

Thanks.
Rafael J. Wysocki - May 4, 2013, 7:19 p.m.
On Thursday, May 02, 2013 05:08:12 PM Tejun Heo wrote:
> On Wed, May 01, 2013 at 06:35:08PM -0700, Colin Cross wrote:
> > Avoid waking up every thread sleeping in read call on an AF_UNIX
> > socket during suspend and resume by calling a freezable blocking
> > call.  Previous patches modified the freezer to avoid sending
> > wakeups to threads that are blocked in freezable blocking calls.
> > 
> > This call was selected to be converted to a freezable call because
> > it doesn't hold any locks or release any resources when interrupted
> > that might be needed by another freezing task or a kernel driver
> > during suspend, and is a common site where idle userspace tasks are
> > blocked.
> 
> Heh, so you are aware of the deadlock possibilities.  Good selection
> of spots.  For all the conversion patches.
> 
>  Acked-by: Tejun Heo <tj@kernel.org>

I wonder if that includes [3/10] (just to get the record straight)?

Rafael
Tejun Heo - May 4, 2013, 8:39 p.m.
Hello, Rafael.

On Sat, May 4, 2013 at 12:19 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> Heh, so you are aware of the deadlock possibilities.  Good selection
>> of spots.  For all the conversion patches.
>>
>>  Acked-by: Tejun Heo <tj@kernel.org>
>
> I wonder if that includes [3/10] (just to get the record straight)?

I think we want the lockdep annotations before these go in. I'll
review Colin's new series later today.

Thanks!

--
tejun
--
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
Colin Cross - May 4, 2013, 10:23 p.m.
On Sat, May 4, 2013 at 1:39 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Rafael.
>
> On Sat, May 4, 2013 at 12:19 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>> Heh, so you are aware of the deadlock possibilities.  Good selection
>>> of spots.  For all the conversion patches.
>>>
>>>  Acked-by: Tejun Heo <tj@kernel.org>
>>
>> I wonder if that includes [3/10] (just to get the record straight)?
>
> I think we want the lockdep annotations before these go in. I'll
> review Colin's new series later today.

Just to make sure the merge order is clear, the two patches I posted
yesterday to reintroduce the lockdep warning in try_to_freeze()
(https://lkml.org/lkml/2013/5/3/488) need to be merged first, then I
will repost this series on top of that one.
--
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
Rafael J. Wysocki - May 5, 2013, 11:23 a.m.
On Saturday, May 04, 2013 03:23:30 PM Colin Cross wrote:
> On Sat, May 4, 2013 at 1:39 PM, Tejun Heo <tj@kernel.org> wrote:
> > Hello, Rafael.
> >
> > On Sat, May 4, 2013 at 12:19 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >>> Heh, so you are aware of the deadlock possibilities.  Good selection
> >>> of spots.  For all the conversion patches.
> >>>
> >>>  Acked-by: Tejun Heo <tj@kernel.org>
> >>
> >> I wonder if that includes [3/10] (just to get the record straight)?
> >
> > I think we want the lockdep annotations before these go in. I'll
> > review Colin's new series later today.
> 
> Just to make sure the merge order is clear, the two patches I posted
> yesterday to reintroduce the lockdep warning in try_to_freeze()
> (https://lkml.org/lkml/2013/5/3/488) need to be merged first, then I
> will repost this series on top of that one.

OK, thanks for the clarification.

Rafael

Patch

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 2db702d..2bcac57 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -114,6 +114,7 @@ 
 #include <linux/mount.h>
 #include <net/checksum.h>
 #include <linux/security.h>
+#include <linux/freezer.h>
 
 struct hlist_head unix_socket_table[2 * UNIX_HASH_SIZE];
 EXPORT_SYMBOL_GPL(unix_socket_table);
@@ -1880,7 +1881,7 @@  static long unix_stream_data_wait(struct sock *sk, long timeo)
 
 		set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
 		unix_state_unlock(sk);
-		timeo = schedule_timeout(timeo);
+		timeo = freezable_schedule_timeout(timeo);
 		unix_state_lock(sk);
 		clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
 	}