Re: [PATCH] [2.6.8-rc2-mm2] More Altix system controller changes

[Andrew Morton <akpm@osdl.org>]

Greg Howard <ghoward@sgi.com> wrote:
>
> Hi Andrew,
> 
> Here's an update to the Altix system conroller communication driver.
> It incorporates some suggestions Christoph made about the last patch
> and fixes a couple of config files.  It's based on 2.6.8-rc2-mm2.
> 
> ...
> -	depends on CONFIG_IA64_SGI_SN2
> +	depends on IA64_SGI_SN2

oop, sorry.

> +scdrv_interrupt(int irq, void *subch_data, struct pt_regs *regs)
> ...
> +	struct subch_data_s *sd = (struct subch_data_s *) subch_data;

Please don't typecast void*'s in this manner: if someone were to change the
type of subch_data, this typecast would cause the useful warning to be
suppressed.


> +	if (status > 0) {
> +		if (status & SAL_IROUTER_INTR_RECV) {
> +			wake_up_all(&sd->sd_rq);
> +		}

Why wake_up_all()?  I think wake_up() is the appropriate function here?

> @@ -396,16 +273,25 @@ scdrv_write(struct file *file, const cha
> 
>  	/* if we failed, and we want to block, then loop */
>  	while (status <= 0) {
> +		DECLARE_WAITQUEUE(wait, current);
> +
>  		if (file->f_flags & O_NONBLOCK) {
>  			spin_unlock(&sd->sd_wlock);
>  			return -EAGAIN;
>  		}
> -		if (scdrv_wait(&sd->sd_wq, &sd->sd_wlock, flags, 1000) < 0) {
> -			/* something went wrong with wait */
> +
> +		add_wait_queue(&sd->sd_wq, &wait);
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		spin_unlock_irqrestore(&sd->sd_wlock, flags);
> +
> +		schedule_timeout(SCDRV_TIMEOUT);
> +
> +		remove_wait_queue(&sd->sd_wq, &wait);
> +		if (signal_pending(current)) {
> +			/* wait was interrupted */
>  			return -ERESTARTSYS;

hmm, if a signal is pending, scdrv_write() will return with the sd->sd_wbs
semaphore still held.  Dead driver, yes?