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;
Subject: Re: [RFC PATCH 2/2] pipe: use pipe busy wait
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     linux-kernel@vger.kernel.org, peterz@infradead.org,
        dhaval.giani@oracle.com, steven.sistare@oracle.com
References: <20180830202458.32579-1-subhra.mazumdar@oracle.com>
 <20180830202458.32579-3-subhra.mazumdar@oracle.com>
 <alpine.DEB.2.21.1809042351220.1416@nanos.tec.linutronix.de>
From:   Subhra Mazumdar <subhra.mazumdar@oracle.com>
Message-ID: <e5f02d36-590d-e15c-01a5-60f1f890fbb2@oracle.com>
Date:   Tue, 4 Sep 2018 17:20:11 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.21.1809042351220.1416@nanos.tec.linutronix.de>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 09/04/2018 02:54 PM, Thomas Gleixner wrote:
> 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.
Makes sense. I will remove it.

Thanks,
Subhra
>
> Thanks,
>
> 	tglx
>