linux-kernel - Re: [take25 1/6] kevent: Description.

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <4565FDED.2050003@redhat.com>
Date:	Thu, 23 Nov 2006 12:00:45 -0800
From:	Ulrich Drepper <drepper@...hat.com>
To:	Evgeniy Polyakov <johnpol@....mipt.ru>
CC:	David Miller <davem@...emloft.net>, Andrew Morton <akpm@...l.org>,
	netdev <netdev@...r.kernel.org>,
	Zach Brown <zach.brown@...cle.com>,
	Christoph Hellwig <hch@...radead.org>,
	Chase Venters <chase.venters@...entec.com>,
	Johann Borck <johann.borck@...sedata.com>,
	linux-kernel@...r.kernel.org, Jeff Garzik <jeff@...zik.org>
Subject: Re: [take25 1/6] kevent: Description.

Evgeniy Polyakov wrote:
> uidx is an index, starting from which there are unread entries. It is
> updated by userspace when it commits entries, so it is 'consumer'
> pointer, while kidx is an index where kernel will put new entries, i.e.
> 'producer' index. We definitely need them both.
> Userspace can only update (implicitly by calling kevent_commit()) uidx.

Right, which is why exporting this entry is not needed.  Keep the 
interface as small as possible.

Userlevel has to maintain its own index.  Just assume kevent_wait 
returns 10 new entries and you have multiple threads.  In this case all 
threads take their turns and pick an entry from the ring buffer.  This 
basically has to be done with something like this (I ignore wrap-arounds 
here to simplify the example):

   int getidx() {
     while (uidx < kidx)
        if (atomic_cmpxchg(uidx, uidx + 1, uidx) == 0)
          return uidx;
     return -1;
   }

Very much simplified but it should show that we need a writable copy of 
the uidx.  And this value at any time must be consistent with the index 
the kernel assumes.

The current ring_uidx value can at best be used to reinitialize the 
userlevel uidx value after each kevent_wait call but this is unnecessary 
at best (since uidx must already have this value) and racy in problem 
cases (what if more than one thread gets woken concurrently with uidx 
having the same value and one thread stores the uidx value and 
immediately increments it to get an index; the second store would 
overwrite the increment).

I can assure you that any implementation I write would not use the 
ring_uidx value.  Only trivial, single-threaded examples like you 
ring_buffer.c could ever take advantage of this value.  It's not worth it.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/