All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Penyaev <rpenyaev@suse.de>
To: Davidlohr Bueso <dbueso@suse.de>
Cc: Jason Baron <jbaron@akamai.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] epoll: remove wrong assert that ep_poll_callback is always called with irqs off
Date: Tue, 08 Jan 2019 17:07:23 +0100	[thread overview]
Message-ID: <2cec506322600c3d2b59f2ca9bcd59dc@suse.de> (raw)
In-Reply-To: <725cdce88418c2ec62ef6014d388dbeb@suse.de>

On 2019-01-08 16:16, Davidlohr Bueso wrote:
> On 2019-01-08 04:42, Roman Penyaev wrote:
>> What we can do:
>> 
>> a) disable irqs if we are not in interrupt.
>> b) revert the patch completely.
>> 
>> David, is it really crucial in terms of performance to avoid double
>> local_irq_save() on Xen on this ep_poll_callback() hot path?
> 
> Note that such optimizations are also relevant for baremetal, ie: x86
> PUSHF + POPF can be pretty expensive because of insn dependencies.
> 
>> 
>> For example why not to do the following:
>> 
>>   if (!in_interrupt())
>>        local_irq_save(flags);
>>   read_lock(ep->lock);
>> 
>> with huge comment explaining performance number.
>> 
>> Or just give up and simply revert the original patch completely
>> and always call read_lock_irqsave().
> 
> Yeah so the reason why I had done the other epoll lock irq
> optimizations was because they were painfully obvious.
> ep_poll_callback(), however is a different beast, as you've
> encountered. I vote for not shooting ourselves in the foot and just
> dropping this patch -- most large performance benefits will come from
> microbenches anyway.

Then I will send another patch which changes read_lock() on 
read_lock_irqsave(),
since simple revert of the original patch won't work.

Thanks.

--
Roman

      reply	other threads:[~2019-01-08 16:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-08 10:01 [PATCH 1/1] epoll: remove wrong assert that ep_poll_callback is always called with irqs off Roman Penyaev
2019-01-08 12:42 ` Roman Penyaev
2019-01-08 15:16   ` Davidlohr Bueso
2019-01-08 16:07     ` Roman Penyaev [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2cec506322600c3d2b59f2ca9bcd59dc@suse.de \
    --to=rpenyaev@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=dbueso@suse.de \
    --cc=jbaron@akamai.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.