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: <45671E16.6060005@redhat.com>
Date:	Fri, 24 Nov 2006 08:30:14 -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:
>> 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.
> 
> I seriously doubt it is simpler than having index provided by kernel.

What has simpler to do with it?  The userlevel code should not modify 
the ring buffer structure at all.  If we'd do this then all operations, 
at least on the uidx field, would have to be atomic operations.  This is 
currently not the case for the kernel side since it's protected by a 
lock for the event queue.  Using the uidx field from userlevel would 
therefore just make things slower.

And for what?  Changing the uidx value would make the commit syscall 
unnecessary.  This might be an argument but it sounds too dangerous. 
IMO the value should be protected by the kernel.

And in any case, the uidx value cannot be updated until the event 
actually has been processed.  But the threads still need to coordinate 
distributing the events from the ring buffer amongst themselves.  This 
will in any case require a second variable.

So, if you want to do away with the commit syscall, keep the uidx value. 
  This also requires that the ring buffer head will always be writable 
(something I'd like to avoid making part of the interface but I'm 
flexible on this).  Otherwise, the ring_uidx element can go away, it's 
not needed and will only make people think about wrong approaches to use it.

> You propose to make uidx shared local variable - it is doable, but it
> is not required - userspace can use kernel's variable, since it is
> updated exactly in the places where that index is changed.

As said above, we always need another variable and uidx is only a 
replacement for the commit call.  Until the event is processed the uidx 
cannot be incremented since otherwise the ring buffer entry might be 
overwritten.

And kernel people of all should be happy to limit the exposure of the 
implementation.  So, leave the problem of keeping track of the tail 
pointer to the userlevel code.

-- 
➧ 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/