linux-kernel - Re: Subject: [PATCH 2/2] priority System V Semaphores

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: <1324526308.5916.123.camel@gandalf.stny.rr.com>
Date:	Wed, 21 Dec 2011 22:58:28 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	raz ben yehuda <raziebe@...il.com>
Cc:	Manfred Spraul <manfred@...orfullife.com>,
	linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	Lior Brafman <LBrafman@...z.com>,
	Torsten Scherer <TScherer@...z.com>,
	Rasty Slutsker <RSlutsker@...z.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: Subject: [PATCH 2/2] priority System V Semaphores

On Wed, 2011-12-21 at 22:48 +0200, raz ben yehuda wrote:
> On Wed, 2011-12-21 at 19:31 +0100, Manfred Spraul wrote:
> > Hi raz,
> > 
> > 
> > On 12/20/2011 11:23 PM, raz ben yehuda wrote:
> > > > From 25aa166505aff2561dd715c927c654d0bbb432ba Mon Sep 17 00:00:00 2001
> > > From: raz<raziebe@...il.com>
> > > Date: Tue, 20 Dec 2011 22:54:56 +0200
> > >
> > >
> > > Add positioning routine find_pos. find_pos returns
> > > the place where to put the sleeper before.
> > > I sort only rt tasks, OTHER policy is pushed back in
> > > queue. I do not distinct between SCHED_RR and SCHED_FIFO
> > > policies and they are treated as a single policy
> > > for the sorting algorithm.
> > >
> > > SETPRIO operates only when user issues a single semop
> > > operation and not an array of opretions.
> > As far as I can see, the advantages of sysvsem are backward 
> > compatibility and the ability to use complex ops.
> > You propose to add a new feature that doesn't work on complex ops.
> > 
> > Are there any apps that use SETPRIO?
> > What is the use case?
> Vxworks is the use case. And there are plenty of companies with
> vxWorks software and in i believe they will migrate sooner or later to
> PreemptRT.  My current company uses old wrapper software that implements
> vxWorks semaphores as system V semaphores. vxWorks semaphores have a priority
> feature which is widely used.
> I will probably change it some time in the future to posix semaphores , but posix 
> semaphores are implemented in glibc with futexes and atomic ops and i rather 
> mess with kernel and not glibc. funny , but true. glibc is harder. 
> 
> > > SETFIFO is the default for backward compatbility.
> > >
> > > Signed-off-by: raz<raziebe@...il.com>
> > > ---
> > >   ipc/sem.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++--
> > >   1 files changed, 49 insertions(+), 2 deletions(-)
> > >e
> > > diff --git a/ipc/sem.c b/ipc/sem.c
> > > index 90dc5a1..921056d 100644
> > > --- a/ipc/sem.c
> > > +++ b/ipc/sem.c
> > > @@ -1343,6 +1343,51 @@ static int get_queue_result(struct sem_queue *q)
> > >   	return error;
> > >   }
> > >
> > > +/*
> > > + * find the best place to put the task sorted by rt prio
> > > +*/
> > > +static struct list_head *find_pos(struct sem *curr, int alter,
> > > +		struct task_struct *p)
> > > +{
> > > +	struct sem_queue *q;
> > > +	struct sem_queue *ret_pos = NULL;
> > > +	struct list_head *tasks_queue =&curr->sem_pending;
> > > +
> > > +	if (!alter)
> > > +		return tasks_queue;
> > > +
> > Tasks that do not alter anything end up first - IMHO correct.
> > 
> > > +	if (!(curr->flags&  PRIO_SEM))
> > > +		return tasks_queue;
> > > +	/*
> > > +	 *  make no effort to sort SCHED_OTHER,
> > > +	 *  just push task to the back of the queue.
> > > +	*/
> > > +	if (!(p->policy == SCHED_FIFO || p->policy == SCHED_RR))
> > > +		return tasks_queue;
> > > +	/*
> > > +	 *  make no distinction between SCHED_FIFO
> > > +	 *  and SCHED_RR policies.
> > > +	 */
> > > +	list_for_each_entry(q, tasks_queue, simple_list) {
> > > +		struct task_struct *t;
> > > +
> > > +		t  = q->sleeper;
> > > +		if (current->rt_priority == t->rt_priority) {
> > > +			/*
> > > +			* push in a FIFO manner
> > > +			* tasks in same priority
> > > +			*/
> > > +			ret_pos = q;
> > > +			continue;
> > > +		}
> > > +		if (current->rt_priority<  t->rt_priority)
> > > +			continue;
> > > +		return&q->simple_list;
> > > +	}
> > Here in the loop, non-alter tasks are evaluated as well.
> > I think that's wrong, it could starve non-alter tasks.
> > e.g. queue:
> > - high prio non-alter
> > - low prio non-alter.
> > 
> > Now a medium prio alter task is added.
> > I think it will end up in the wrong position (before the low-prio 
> > non-alter task), correct?
> hammm.. correct. thanks. I did not check non-alter tasks at all. The entire queue might be a total mess. 
> I will sort non alter tasks as well.
> Also, this patch is missing scenario where a task priority is changed whilst it is waiting. 
> I fixed that already. 
> 
> Who maintains this code ? 
> 

It's mainly core kernel code and only changes when true changes are
made. An update could be done to extend its use as long as it doesn't
break any tools in the process. IOW, it must stay backward compatible.

-- Steve


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