All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Frederic Weisbecker <frederic@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	Dave Chinner <david@fromorbit.com>,
	Valentin Schneider <vschneid@redhat.com>,
	Leonardo Bras <leobras@redhat.com>,
	Yair Podemsky <ypodemsk@redhat.com>, P J P <ppandit@redhat.com>
Subject: Re: [PATCH] fs/buffer.c: disable per-CPU buffer_head cache for isolated CPUs
Date: Fri, 4 Aug 2023 20:54:37 -0300	[thread overview]
Message-ID: <ZM2PvQJd7kRyWnAZ@tpad> (raw)
In-Reply-To: <ZM11z1Jxqrwk47e9@lothringen>

On Sat, Aug 05, 2023 at 12:03:59AM +0200, Frederic Weisbecker wrote:
> On Tue, Jun 27, 2023 at 05:08:15PM -0300, Marcelo Tosatti wrote:
> > 
> > For certain types of applications (for example PLC software or
> > RAN processing), upon occurrence of an event, it is necessary to
> > complete a certain task in a maximum amount of time (deadline).
> > 
> > One way to express this requirement is with a pair of numbers,
> > deadline time and execution time, where:
> > 
> >         * deadline time: length of time between event and deadline.
> >         * execution time: length of time it takes for processing of event
> >                           to occur on a particular hardware platform
> >                           (uninterrupted).
> > 
> > The particular values depend on use-case. For the case
> > where the realtime application executes in a virtualized
> > guest, an IPI which must be serviced in the host will cause
> > the following sequence of events:
> > 
> >         1) VM-exit
> >         2) execution of IPI (and function call)
> >         3) VM-entry
> > 
> > Which causes an excess of 50us latency as observed by cyclictest
> > (this violates the latency requirement of vRAN application with 1ms TTI,
> > for example).
> > 
> > invalidate_bh_lrus calls an IPI on each CPU that has non empty
> > per-CPU cache:
> > 
> >         on_each_cpu_cond(has_bh_in_lru, invalidate_bh_lru, NULL, 1);
> > 
> > The performance when using the per-CPU LRU cache is as follows:
> > 
> >  42 ns per __find_get_block
> >  68 ns per __find_get_block_slow
> > 
> > Given that the main use cases for latency sensitive applications
> > do not involve block I/O (data necessary for program operation is 
> > locked in RAM), disable per-CPU buffer_head caches for isolated CPUs.

Hi Frederic,

> So what happens if they ever do I/O then? Like if they need to do
> some prep work before entering an isolated critical section?

Then instead of going through the per-CPU LRU buffer_head cache
(__find_get_block), isolated CPUs will work as if their per-CPU
cache is always empty, going through the slowpath 
(__find_get_block_slow). The algorithm is:

/*
 * Perform a pagecache lookup for the matching buffer.  If it's there, refresh
 * it in the LRU and mark it as accessed.  If it is not present then return
 * NULL
 */
struct buffer_head *
__find_get_block(struct block_device *bdev, sector_t block, unsigned size)
{
        struct buffer_head *bh = lookup_bh_lru(bdev, block, size);

        if (bh == NULL) {
                /* __find_get_block_slow will mark the page accessed */
                bh = __find_get_block_slow(bdev, block);
                if (bh)
                        bh_lru_install(bh);
        } else
                touch_buffer(bh);

        return bh;
}
EXPORT_SYMBOL(__find_get_block);

I think the performance difference between the per-CPU LRU cache
VS __find_get_block_slow was much more significant when the cache 
was introduced. Nowadays its only 26ns (moreover modern filesystems 
do not use buffer_head's).

> Thanks.

Thank you for the review.

> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > diff --git a/fs/buffer.c b/fs/buffer.c
> > index a7fc561758b1..49e9160ce100 100644
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -49,6 +49,7 @@
> >  #include <trace/events/block.h>
> >  #include <linux/fscrypt.h>
> >  #include <linux/fsverity.h>
> > +#include <linux/sched/isolation.h>
> >  
> >  #include "internal.h"
> >  
> > @@ -1289,7 +1290,7 @@ static void bh_lru_install(struct buffer_head *bh)
> >  	 * failing page migration.
> >  	 * Skip putting upcoming bh into bh_lru until migration is done.
> >  	 */
> > -	if (lru_cache_disabled()) {
> > +	if (lru_cache_disabled() || cpu_is_isolated(smp_processor_id())) {
> >  		bh_lru_unlock();
> >  		return;
> >  	}
> > @@ -1319,6 +1320,10 @@ lookup_bh_lru(struct block_device *bdev, sector_t block, unsigned size)
> >  
> >  	check_irqs_on();
> >  	bh_lru_lock();
> > +	if (cpu_is_isolated(smp_processor_id())) {
> > +		bh_lru_unlock();
> > +		return NULL;
> > +	}
> >  	for (i = 0; i < BH_LRU_SIZE; i++) {
> >  		struct buffer_head *bh = __this_cpu_read(bh_lrus.bhs[i]);
> >  
> > 
> 
> 


  reply	other threads:[~2023-08-04 23:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27 20:08 [PATCH] fs/buffer.c: disable per-CPU buffer_head cache for isolated CPUs Marcelo Tosatti
2023-07-26 14:31 ` Marcelo Tosatti
2023-07-27  9:18   ` Christian Brauner
2023-07-28 19:35     ` Marcelo Tosatti
2023-08-04 22:03 ` Frederic Weisbecker
2023-08-04 23:54   ` Marcelo Tosatti [this message]
2023-08-10 10:36     ` Frederic Weisbecker
2023-08-10 11:59 ` Christian Brauner

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=ZM2PvQJd7kRyWnAZ@tpad \
    --to=mtosatti@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=david@fromorbit.com \
    --cc=frederic@kernel.org \
    --cc=hch@lst.de \
    --cc=leobras@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ppandit@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=vschneid@redhat.com \
    --cc=willy@infradead.org \
    --cc=ypodemsk@redhat.com \
    /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.