Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 4 Sep 2018 23:54:59 +0200 (CEST)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     subhra mazumdar <subhra.mazumdar@oracle.com>
cc:     linux-kernel@vger.kernel.org, peterz@infradead.org,
        dhaval.giani@oracle.com, steven.sistare@oracle.com
Subject: Re: [RFC PATCH 2/2] pipe: use pipe busy wait
In-Reply-To: <20180830202458.32579-3-subhra.mazumdar@oracle.com>
Message-ID: <alpine.DEB.2.21.1809042351220.1416@nanos.tec.linutronix.de>
References: <20180830202458.32579-1-subhra.mazumdar@oracle.com> <20180830202458.32579-3-subhra.mazumdar@oracle.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 30 Aug 2018, subhra mazumdar wrote:
>  
> +void pipe_busy_wait(struct pipe_inode_info *pipe)
> +{
> +	unsigned long wait_flag = pipe->pipe_wait_flag;
> +	unsigned long start_time = pipe_busy_loop_current_time();
> +
> +	pipe_unlock(pipe);
> +	preempt_disable();
> +	for (;;) {
> +		if (pipe->pipe_wait_flag > wait_flag) {
> +			preempt_enable();
> +			pipe_lock(pipe);
> +			return;
> +		}
> +		if (pipe_busy_loop_timeout(pipe, start_time))
> +			break;
> +		cpu_relax();
> +	}
> +	preempt_enable();

You are not really serious about busy looping with preemption disabled?

That's just wrong. Why do you want to block others from getting on the CPU
if there is nothing in the pipe?

There is no point in doing so, really. If the wait loop is preempted
because there is more important work to do, then it will come back and
either see new data, or leave due to wait time reached.

Thanks,

	tglx