netdev - Re: [PATCH] GENETLINK: Global lock refined to family granularity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 31 Mar 2008 14:26:48 +0200
From:	Thomas Graf <tgraf@...g.ch>
To:	Richard MUSIL <richard.musil@...com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] GENETLINK: Global lock refined to family granularity

* Richard MUSIL <richard.musil@...com> 2008-03-31 13:21
> It takes two locks, instead of one. I do not see, how it could be less
> (in terms of synchronization primitives - but not necessarily mutexes)
> if I want that level of granularity. Might be more efficient with RCU
> lists (but I do not know), but complexity remains the same.

The synchronization mutex in genl_recv() remains and you add an
additional global mutex genl_fam_lock and a per family lock. That's
2 vs 6 lock operations. It is possible to use RCU to manage the
family, ops and groups list and make the processing path lockless.
All you have to do is synchronize the family unregister function to
message processing as dumping can't disable preempt. Given this, your
patch would be a lot more acceptable to me as the biggest cost would
be reduced and the cost vs gain ratio would be a lot better.

> In that case, nothing can be done. Of course, trying to unregister
> family form its own handler could lead to either crash or deadlock. The
> point was, it would not deadlock when manipulating other family - which
> happens with current implementation.
> Also, it would allow processing of different messages in parallel as
> long as they are belonging to different families. Right now, the
> processing is blocked as long as any single message is processed.

The old behaviour is well defined and obvious. If you allow manipulation
during message processing then allow it for all objects not just some.

I'm all for parallel processing. It has been requested for some time now
by various people but so far nobody got around implementing it. It would
be a very legitimate reason to complicate locking and would even be
worth some additional locking costs. I'm all for this.

> I am not convinced about that. Does it mean you suggest that any
> genetlink message handler should by default offload processing to
> workqueue, so it won't block any other message processing from any other
> family?

I meant deferring the registration of operations to after message
processing using workqueues. Much like it is done for many other things
which can't be done in current locking context.

> You right it does not. Should I look into it, or was your answer
> definitive no?

I'm not against the idea, I just feel that the gains in your patch in
its current form do not justify the additional locking and complextity
costs. Combined with parallel processing and RCU lists it would be a
no brainer and probably merged instantly.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html