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: <20060802010915.GC22589@redhat.com>
Date:	Tue, 1 Aug 2006 21:09:15 -0400
From:	Dave Jones <davej@...hat.com>
To:	Nish Aravamudan <nish.aravamudan@...il.com>
Cc:	arjan@...radead.org, Linux Kernel <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...l.org>
Subject: Re: deprecate and convert some sleep_on variants.

On Tue, Aug 01, 2006 at 01:20:28PM -0500, Nish Aravamudan wrote:

 > >+  wait_queue_t __wait;
 > >+
 > >+  init_waitqueue_entry(&__wait, current);
 > >
 > >   spin_lock_irqsave(&Controller->queue_lock, flags);
 > >   while ((Command = DAC960_AllocateCommand(Controller)) == NULL)
 > >@@ -6314,11 +6317,18 @@ static boolean DAC960_V2_ExecuteUserComm
 > >                                        .SegmentByteCount =
 > >            CommandMailbox->ControllerInfo.DataTransferSize;
 > >          DAC960_ExecuteCommand(Command);
 > >+         add_wait_queue(&Controller->CommandWaitQueue, &__wait);
 > >+         set_current_state(TASK_UNINTERRUPTIBLE);
 > 
 > Could this use prepare_to_wait()

Maybe, though I'd rather not do that conversion with the hardware to test it.
sidenote: prepare_to_wait() and friends could really use some kerneldoc explaining
their purpose rather than their internal workings.

 > >          while 
 > >          (Controller->V2.NewControllerInformation->PhysicalScanActive)
 > >            {
 > >              DAC960_ExecuteCommand(Command);
 > >-             sleep_on_timeout(&Controller->CommandWaitQueue, HZ);
 > >+             schedule_timeout(HZ);
 > >+             set_current_state(TASK_UNINTERRUPTIBLE);
 > 
 > and schedule_timeout_uninterruptible() (which is redundant for the
 > first invocation, I suppose)

Makes sense.

 > >+         current->state = TASK_RUNNING;
 > >+         remove_wait_queue(&Controller->CommandWaitQueue, &__wait);
 > 
 > and finish_wait()?
 > 
 > Same for ibmtr.c ?

Same comments as above.

 > Also, would these changes:
 > 
 > >diff -urNp --exclude-from=/home/davej/.exclude 
 > >linux-1060/include/linux/wait.h linux-1070/include/linux/wait.h
 > >--- linux-1060/include/linux/wait.h
 > >+++ linux-1070/include/linux/wait.h
 > 
 > Be better in a separate patch?

A split-up patchset would for sure make sense for committing upstream.
Though, at least each file touched here is a separate cset.

		Dave

-- 
http://www.codemonkey.org.uk
-
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ